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

TypeError: Class constructor App cannot be invoked without 'new' #1751

Closed
FezVrasta opened this issue Mar 7, 2017 · 15 comments
Closed

TypeError: Class constructor App cannot be invoked without 'new' #1751

FezVrasta opened this issue Mar 7, 2017 · 15 comments

Comments

@FezVrasta
Copy link
Contributor

FezVrasta commented Mar 7, 2017

If you are reporting a bug, please fill in below. Otherwise feel free to remove this template entirely.

Can you reproduce the problem with latest npm?

Yes
Yes

Description

Trying to use React CSS Modules inside create-react-app, only during Jest tests, leads to:

TypeError: Class constructor App cannot be invoked without 'new'

Expected behavior

The code should work fine, like it happens when I use yarn start

Actual behavior

I get the error reported above.

Environment

Run these commands in the project folder and fill in their results:

  1. npm ls react-scripts (if you haven’t ejected): 0.9.4
  2. node -v: 7.5.0
  3. npm -v: 4.1.2

Then, specify:

  1. Operating system: macOS Sierra
  2. Browser and version: no browser

Reproducible Demo

This is a super simple repository created with create-react-app:
https://github.com/FezVrasta/create-react-app-css-modules-bug

Just run yarn test to see the error.
The only edited file is:
https://github.com/FezVrasta/create-react-app-css-modules-bug/blob/master/src/App.js

Where I added react-css-modules

Note that this error happens only when I use create-react-app as base, in other repositories setup from scratch it works fine, but I can't understand why.

@FezVrasta
Copy link
Contributor Author

FezVrasta commented Mar 7, 2017

I just noticed that if as .babelrc presets I use:

"presets": ["es2015", "stage-2", "react"],

instead of the one of create-react-app, it works.

It must be something in the react-app preset that gets injected only in test environment.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

While unrelated to your question, I am a bit confused about how you expect react-css-modules to work. Doesn't it require actual CSS Modules? We don't support them out of the box, you'd have to eject for that.

@FezVrasta
Copy link
Contributor Author

Yes I actually eject it, but I could reproduce the same problem without ejecting it and I think it's enough to proof that there's something wrong in the create-react-app code and not in mine.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

The problem here is likely that react-css-modules uses extends via Babel, but AFAIK you can't extend a native class with a Babel-defined class due to the code Babel generates. Since we use native Node classes in Jest configuration, and your component is a native class, extending from it via Babel breaks.

While we could fix it by always applying the class transform, we gotta move to real ES classes at some point, so it’s still a time bomb. I would just suggest avoiding libraries that try to inherit from your classes, or bringing it up with those libraries to not use the Babel class transform.

I hope this helps!

@gaearon gaearon closed this as completed Mar 7, 2017
@FezVrasta
Copy link
Contributor Author

I'd have preferred to get a fix here, because this project is already very limiting and adding this additional limitation seems simply to much.

Also, it doesn't make sense that the same code will work in browser but not during tests, within the same project.

But I guess it's up to you.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

What kind of fix are you proposing? It is not very clear to me. As I said, any kind of fix would be a time bomb because it would still stop working as soon as you want to switch to native classes in the browser (which you'll probably want to in a couple of years).

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

because this project is already very limiting and adding this additional limitation seems simply to much.

What additional limitation are you referring to? CSS modules don’t work out of the box anyway. If you eject, you might as well make your own Babel config that enables Babel class transform on Node too. But keep in mind it will make your tests slower, and will keep your app tied to Babel class transform. Because even if you want to disable it in the future, you’d bump into the same issue.

This is not really our issue, but rather Babel output being incompatible with the native classes. You can bring it up in react-css-modules and voice your concerns in babel/babel#4480 (since react-css-modules uses Babel for compilation, and the original issue is Babel’s). Unfortunately I don’t have a good solution for you, which is why I recommend to avoid libraries that rely on inheritance from user classes if possible.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

Also, now that I think of it, I don't really see why you'd want to apply react-css-modules in tests. So you might as well mock it to return component untouched:

jest.mock('react-css-modules', () => Component => Component);

This would probably circumvent the problem.

@FezVrasta
Copy link
Contributor Author

FezVrasta commented Mar 7, 2017

If I don't apply classes in tests, I miss a lot of logic of my app which executes code to decide which class apply. The easier way to make sure a component is properly tested is to use the exact same code that will go to production.

About the extension of native classes, I don't understand why you say that it will break in the future, you can extend native classes in Chrome at the moment:
image

Am I missing something?

About the solution, just enable the class transform on Jest, doing so you are testing exactly the same code that you ship to production.
I don't think that at Facebook you test a code different from the one you ship to production, right? Otherwise, if something is wrong in the "different part" between tests and production, you will miss it.

About the limitations, you can't use PostCSS, you can't define custom settings, I know, there's a reason, but at least, make what you provide work as an user expects and don't add further limitations to it.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

If I don't apply classes in tests, I miss a lot of logic of my app which executes code to decide which class apply. The easier way to make sure a component is properly tested is to use the exact same code that will go to production.

It won’t be exact same code anyway. The environment is different, the VM is different, etc. What you describe sounds like integration testing, and I fully agree you should add integration tests (e.g. Selemium) to your app.

About the extension of native classes, I don't understand why you say that it will break in the future, you can extend native classes in Chrome at the moment:

I’m not sure if you read the linked issue, but it’s not extending native classes that’s broken per se, but extending them using Babel transform. If A is native, but B is written with Babel class transform, and B extends A, it will break. This is a known Babel issue (I linked to it), and Babel likely won’t fix it because of performance concerns with the fully compatible solution. This doesn’t come up very often because it works the other way around. So it’s only a problem if a library uses Babel and that library happens to extend your class which might be native.

@gaearon
Copy link
Contributor

gaearon commented Mar 7, 2017

This really is an issue with react-css-modules, and you should file it with them. Nobody who uses native ES classes (e.g. in Chrome) will be able to use that library. Adding workarounds here doesn’t solve that problem. I think it’s useful if CRA acts as a forcing function to get more maintainers familiar with how their projects work once their users depend less on Babel.

@FezVrasta
Copy link
Contributor Author

Okay I see your point, thank you.

@vcarel
Copy link

vcarel commented Sep 18, 2017

I'll probably say something stupid, but why not removing react-css-modules from create-react-app or make it opt-in?
I don't see the related issue to be fixed anytime soon: https://github.com/gajus/react-css-modules/issues/196

I'm not using CSS in JS, but aren't there any other solution? On my end, i'm disappointed to not being able to extend ES6 classe because of a plugin I don't use.

One of the benefits of create-react-app is that things should go smooth. A buggy dependency impacts the quality of the whole project, and I don't think you should let that happen (nevertheless, it's an edge case)

@gaearon
Copy link
Contributor

gaearon commented Sep 18, 2017

I'll probably say something stupid, but why not removing react-css-modules from create-react-app or make it opt-in?

I don't understand. Create React App does not include this plugin.

If you experience an error with this message please provide an example reproducing it.

@vcarel
Copy link

vcarel commented Sep 18, 2017

@gaearon My apologizes, I didn't read properly the first comment. I experience the same problem, indeed, but found out it's because of React Router.

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

No branches or pull requests

3 participants