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

Material UI breaks all tests on react v16 #8643

Closed
1 task done
austinh opened this issue Oct 10, 2017 · 12 comments
Closed
1 task done

Material UI breaks all tests on react v16 #8643

austinh opened this issue Oct 10, 2017 · 12 comments
Labels
bug 🐛 Something doesn't work v0.x

Comments

@austinh
Copy link

austinh commented Oct 10, 2017

I upgraded to React v16 and chai-enzyme v3, and any component test that I have that utilizes Material ui (particularly MuiThemeProvider) that is mounted throws various errors like the following:

TypeError: Cannot set property 'stroke' of undefined
TypeError: Cannot set property 'display' of undefined

  • I have searched the issues of this repository and believe that this is not a duplicate.

I tried to create a codepen to make an example but i was unable to setup enzyme test runner.

Expected Behavior

React v16 and material ui together should not break all tests.

Current Behavior

Material UI combined with enzyme 3 and react v16 break all tests
Note: this does not effect the actual components in a browser, only in the test environment when mounted via enzyme, and that mounted component also happens to use MuiThemeProvider.

Context

Cannot upgrade to react v16, this is the last thing I need to fix to get my tests running and finally upgrade.

Tech Version
Material-UI v0.19.4
React v16.0.0
enzyme v3.0.0
@oliviertassinari
Copy link
Member

Do you have a stacktrace?

@oliviertassinari
Copy link
Member

Oh, it's for the legacy version. Sorry, I can't help for this one.

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Oct 10, 2017
@austinh
Copy link
Author

austinh commented Oct 10, 2017

And is there a migration guide for v1? Many of us have code that is utilizing v0.19. I would love to upgrade to v1, but it also breaks all my site :( v1 is not even released (its still in beta).

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 10, 2017

@austinh We still accept PR fixes. If you find a fix, we will definitely review it :). Last one was this morning #8634. What I'm saying is that I have low incentive in spending time on v0.x issues.

And is there a migration guide for v1?

There is one that needs to be completed.

@hyalen
Copy link

hyalen commented Oct 11, 2017

My tests had this same error message. If you're using MomentJS, the problem probably comes from that framework. The last version of Moment must be downgraded to "2.18.1".

For me, the solution was:

  1. delete node-modules folder
  2. lock all the dependencies from your package.json
  3. type npm install
  4. be happy

@jwaldrip
Copy link

@oliviertassinari - Definitely a negative behavior to close requests on what is still considered the stable version of material-ui. This is something you should be continuing to fix and maintain until v1.0 has been released. In the mean time, I too am running into this issue with 0.19.4 and would love to see a fix.

@jwaldrip
Copy link

jwaldrip commented Oct 17, 2017

@hyalen your fix did not work, I downgraded moment, reinstalled modules, and the error persisted.

  8) <CardIcon />
       sets the style to the icon when is neither isSelected nor isDisabled:
     TypeError: Cannot set property 'display' of undefined
      at Object.setValueForStyles (node_modules/react-dom/cjs/react-dom.development.js:3075:26)
      at setInitialDOMProperties (node_modules/react-dom/cjs/react-dom.development.js:5958:31)
      at setInitialProperties (node_modules/react-dom/cjs/react-dom.development.js:6151:5)
      at finalizeInitialChildren (node_modules/react-dom/cjs/react-dom.development.js:16917:5)
      at completeWork (node_modules/react-dom/cjs/react-dom.development.js:10923:19)
      at completeUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:12481:18)
      at performUnitOfWork (node_modules/react-dom/cjs/react-dom.development.js:12580:14)
      at workLoop (node_modules/react-dom/cjs/react-dom.development.js:12682:28)
      at HTMLUnknownElement.callCallback (node_modules/react-dom/cjs/react-dom.development.js:1299:14)
      at invokeEventListeners (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:193:27)
      at HTMLUnknownElementImpl._dispatch (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:119:9)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:82:17)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/nodes/HTMLElement-impl.js:41:27)
      at HTMLUnknownElement.dispatchEvent (node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:143:21)
      at Object.invokeGuardedCallbackDev (node_modules/react-dom/cjs/react-dom.development.js:1338:16)
      at invokeGuardedCallback (node_modules/react-dom/cjs/react-dom.development.js:1195:27)
      at performWork (node_modules/react-dom/cjs/react-dom.development.js:12800:7)
      at scheduleUpdateImpl (node_modules/react-dom/cjs/react-dom.development.js:13185:19)
      at scheduleUpdate (node_modules/react-dom/cjs/react-dom.development.js:13124:12)
      at scheduleTopLevelUpdate (node_modules/react-dom/cjs/react-dom.development.js:13395:5)
      at Object.updateContainer (node_modules/react-dom/cjs/react-dom.development.js:13425:7)
      at node_modules/react-dom/cjs/react-dom.development.js:17105:19
      at Object.unbatchedUpdates (node_modules/react-dom/cjs/react-dom.development.js:13256:14)
      at renderSubtreeIntoContainer (node_modules/react-dom/cjs/react-dom.development.js:17104:17)
      at Object.render (node_modules/react-dom/cjs/react-dom.development.js:17129:12)
      at Object.render (node_modules/enzyme-adapter-react-16/build/ReactSixteenAdapter.js:216:50)
      at new ReactWrapper (node_modules/enzyme/build/ReactWrapper.js:98:16)
      at mount (node_modules/enzyme/build/mount.js:19:10)
      at mountWithMui (test/mui-enzyme.js:10:37)
      at validateStyle (src/card-selector/card-icon/card-icon.component.spec.jsx:25:19)
      at Context.<anonymous> (src/card-selector/card-icon/card-icon.component.spec.jsx:21:3)

@jwaldrip
Copy link

HA, I found the issue... JSDOM has not implemented SVG and therefor breaks fibers rendering within JSDOM. A fix should be in soon: jsdom/jsdom#2011.

@oliviertassinari
Copy link
Member

@jwaldrip This is open source, I'm not in the business of philanthropy. I work and share the work on the issues I need to move forward. It doesn't mean you are entitled to free work from me.

Still, I have some incentives in keeping the v0.x branch in a reasonable state. I will invest time in improving the migration guide to v1.x. My biggest priority for the v0.x branch is fixing #8040.

@austinh
Copy link
Author

austinh commented Oct 18, 2017

No one is expecting you yourself to fix it, but closing the issue entirely means other people won’t see this issue, and are less likely to also offer fixes. I looked into fixing it myself, but I don’t have nearly the expertise in this libraries internal workings to even know where to start. I opened this issue with the hope that someone (not even necessarily you) would be able to help.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 18, 2017

@austinh Yes, it's on purpose, the priority for ongoing work is the v1-beta branch. Closing this issue help keeping the focus on that branch.

@mkawczynski07
Copy link

mkawczynski07 commented Nov 20, 2017

@jwaldrip you could use require-hacker and add hook to all svg imports. To do that creat file e.g. mocha.loader.js:

import requireHacker from 'require-hacker';
const FAKE_COMPONENT_BODY = '() => null';

requireHacker.global_hook('svg', path => {
  if (path.includes('svg-icons')) {
    return {source: `module.exports = ${FAKE_COMPONENT_BODY}`, path};
  }
});

and run your test with

mocha.cmd test-file.spec.js --require mocha.loader.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work v0.x
Projects
None yet
Development

No branches or pull requests

5 participants