Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shallow transitions #77

Open
beliolfa opened this issue Jul 4, 2017 · 35 comments
Open

Shallow transitions #77

beliolfa opened this issue Jul 4, 2017 · 35 comments

Comments

@beliolfa
Copy link
Contributor

beliolfa commented Jul 4, 2017

Is there a way to bypass this exception when component has transitions?

TypeError: Cannot read property 'split' of undefined
    getTransitionInfo (node_modules/vue/dist/vue.runtime.common.js:6145:58)
    whenTransitionEnds (node_modules/vue/dist/vue.runtime.common.js:6115:13)
    Timeout._onTimeout (node_modules/vue/dist/vue.runtime.common.js:6342:11)
@eddyerburgh
Copy link
Owner

eddyerburgh commented Jul 4, 2017

JSDOM doesn't support transitions yet.

Adding this code will stop the error:

const { getComputedStyle } = window
window.getComputedStyle = function getComputedStyleStub(el) {
	return {
		...getComputedStyle(el),
		transitionDelay: '',
		transitionDuration: '',
		animationDelay: '',
		animationDuration: '',
	}
}

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 4, 2017

No problem with the transition test, but how about the rest of the tests that were passing before adding the transition wrapper? There is no way to override the transition component to fake its behavior? I mean, render the slot inside a normal div?

@eddyerburgh
Copy link
Owner

Not at the moment, but there should be.

I'll have a think about it

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 5, 2017

I could make it work with this approach

import test from 'ava';
import { shallow } from 'avoriaz';
import Vue from 'vue';
import Component from './Component';

const fakeTransitions = () => {
  window.getComputedStyle = () => {
    return {
      transitionDelay: '',
      animationDelay: '',
      transitionDuration: '',
      animationDuration: '',
    };
  };

  Vue.component('transition', {
    render(createElement) {
      return createElement(
        'div',
        this.$slots.default
      );
    },
  });
};

test('it does not throw any exception', (t) => {
  shallow(Component);
  fakeTransitions();

  t.true(true);
});

If you want I may prepare some PR

@eddyerburgh
Copy link
Owner

Yes, that's a great idea! I'd really appreciate a PR

Could you make it a function called stubTransitions.

It should take a Vue instance as an optional argument, and default to using the global instance. That way you avoid stubbing transitions for all tests by passing it a custom instance. You can see a similar implementation in mount.

@eddyerburgh
Copy link
Owner

@disitec are you planning on doing this in a PR? I'll work on it some time if you're unable to.

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 6, 2017

I am a bit busy this days. I'll try doing the PR tomorrow. Anyways is up to you if you have the time!

@eddyerburgh
Copy link
Owner

I'm also busy 😛

I was thinking of working on it next week, but if you can do it before then that would be awesome!

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 7, 2017

I'm also trying to stub transition-group but I am a bit stucked in this warning

TypeError: Cannot read property 'length' of undefined
    at VueComponent.updated (/Users/belio/Code/teameq/frontend_vue/node_modules/vue/dist/vue.runtime.common.js:6994:18)

The implementation is the same as with transition:

Vue.component('transition-group', {
    render(createElement) {
      return createElement(
        'div',
        this.$slots.default,
      );
    },
  });

At this point, children is undefined. Maybe you can help me in the way to pass an empty array to the constructor to bypass the warning.

//vue.runtime.common.js:6994:18

var children = this.prevChildren;
    var moveClass = this.moveClass || ((this.name || 'v') + '-move');
    if (!children.length || !this.hasMove(children[0].elm, moveClass)) {
      return
    }

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 9, 2017

Hello @eddyerburgh I was writting some tests to reproduce the issue in your repo but it didn't happen in mocha. How should i proceed to implement this PR?

P.S. I am a bit newby contributing :). I do my best.

@eddyerburgh
Copy link
Owner

Are you using the same code that threw the error you posted in this issue?

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 9, 2017

Yes, a reproduction of that. I wrap a v-if / else into a transition. AVA throws the split of undefined error (in my project). Mocha does not.

@eddyerburgh
Copy link
Owner

eddyerburgh commented Jul 9, 2017

I think the difference will be the environmen they are ran in, not the test runner that's used.

I managed to reproduce the same error a few months ago in mocha, with the test:unit script.

Does the test pass when you run npm run test:unit?

@beliolfa
Copy link
Contributor Author

beliolfa commented Jul 9, 2017

Yep. That's the one I'm using. I get green.

@eddyerburgh
Copy link
Owner

What version of JSDOM were you using in the project that threw the original error?

@beliolfa
Copy link
Contributor Author

9.12.0

@eddyerburgh
Copy link
Owner

Can you post the test and the component?

@beliolfa
Copy link
Contributor Author

I created a gist with the raw version of them. Please let me know when you have the code in order to delete it. It is a commercial App.

https://gist.github.com/disitec/cc84c49bff39d27dbd7ad3f2bc762584

Thanks!

@eddyerburgh
Copy link
Owner

eddyerburgh commented Jul 10, 2017

Which test was failing? Or was it all of them?

@beliolfa
Copy link
Contributor Author

All tests are passing. But throw warnings in console that I am triying to avoid. Please, tweak the component if you get errors due to the missing Question.vue and mixin.

@eddyerburgh
Copy link
Owner

Ok, I've copied the gist so you can delete.

I'll have to look at this later tonight - I'm pretty busy today. I also can't get a failing test in avoriaz, but am unsure why as this was previously a problem 😕

@eddyerburgh
Copy link
Owner

Hi @disitec , sorry I left you high and dry here. Did you manage to find out the cause of the warnings?

I've been super busy recently and haven't managed to find the time to to debug, but this feature sounded really interesting.

@beliolfa
Copy link
Contributor Author

Don't worry! I'm aware that you are working very hard in the official Vue testing. I'm looking forward to try out.

I've been busy as well but maybe I can start looking into it next week on.

@Zizaco
Copy link

Zizaco commented Aug 10, 2017

Looking forward to a solution.
Currently we have this nasty workaround 😓

/**
 * Monkey patch the given method replacing all 'transition' string for 'dev'.
 *
 * The DOM implementation of Node (JSDOM) don't behave the same way that the
 * browser's when calling 'getComputedStyle' of an element. Due to that, VueJS
 * crashes whenever there is an '<transition>' element within a component.
 *
 * patchTemplate will can replace the 'transition' elements of Vue shadow DOM
 * in order to be able to render the templates in Node.js
 *
 * @see https://github.com/eddyerburgh/avoriaz/issues/77
 * @param  {function} func 'render' function to be patched
 * @return {function} patched function
 */
function patchTemplate(func) {
  let transitionLessTemplate = func.toString().replace(/\'transition\'/g, '\'div\'')
  return eval(`(${transitionLessTemplate})`)
}

/**
 * Wraps avoriaz mount in order to patchTemplate
 * @see  https://eddyerburgh.gitbooks.io/avoriaz/content/api/mount/
 */
var mount = function (...params) {
  if (typeof params[0].render === 'function') {
    params[0].render = patchTemplate(params[0].render)
  }
  return avoriaz.mount(...params)
}

export default { mount, shallow }
export { mount, shallow }

@eddyerburgh
Copy link
Owner

I don't think a solution is being actively developed. I can look at it in a few weeks, but if you want to make a PR I would really appreciate it! 😀

@beliolfa
Copy link
Contributor Author

beliolfa commented Sep 6, 2017

I am ready to make the PR but which kind of test should I write? This issue does not happen in mocha. As we talked in Discord I propose an API such as import { stubTransitions } from 'avoriaz'

So when stubTransitions(); is called this code is executed overriding the undefined.

window.getComputedStyle = () => ({
    transitionDelay: '',
    animationDelay: '',
    transitionDuration: '',
    animationDuration: '',
  })

@eddyerburgh
Copy link
Owner

Where does it happen? I thought it would happen when running in mocha-webpack, because that uses JSDOM.

@beliolfa
Copy link
Contributor Author

beliolfa commented Sep 6, 2017

I cannot reproduce the issue in mocha-webpack. In my private project it always happens using AVA.

@beliolfa
Copy link
Contributor Author

beliolfa commented Sep 6, 2017

We had this discussion in July :D

@eddyerburgh
Copy link
Owner

Ok, I think you can just have a test that calls it. I have a feeling overwriting getComputedStyle will break wrapper.hasStyle. Maybe you could also add a call in hasStyle.spec.js.

@beliolfa
Copy link
Contributor Author

beliolfa commented Sep 7, 2017

You were right. It breaks hasStyle. I'll do a WIP PR so you can take a look ok?

@kb-coder
Copy link

Is there any update on this? I'm running into the same issue with transition-group and transition.

@acacha
Copy link

acacha commented Nov 9, 2017

Related info? vuejs/vue-test-utils#52

@acacha
Copy link

acacha commented Nov 9, 2017

It works for me using vue-test-utils and shallow instead of mount when mounting component.

@mleg
Copy link

mleg commented Jul 25, 2018

Like disitec, I managed to work the problem around by brute force stubbing getComputedStyle function.
Just keep in mind you can amend jsdom implementation instead of replacing it:

const { getComputedStyle } = window
window.getComputedStyle = function getComputedStyleStub(el) {
	return {
		...getComputedStyle(el),
		transitionDelay: '',
		transitionDuration: '',
		animationDelay: '',
		animationDuration: '',
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants