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

Missing export should be a compile error #1559

Closed
gaearon opened this issue Feb 15, 2017 · 15 comments
Closed

Missing export should be a compile error #1559

gaearon opened this issue Feb 15, 2017 · 15 comments
Milestone

Comments

@gaearon
Copy link
Contributor

gaearon commented Feb 15, 2017

This is only relevant in master which uses Webpack 2.

If I change src/index.js from

import App from './App';

to

import {App} from './App';

I'm getting:

screen shot 2017-02-15 at 00 40 43

The screen is blank (because of the runtime error).

Notice how the real problem is at the very bottom (missing export), yet it looks like the first two are the important ones.

The terminal output is also very restrained (because it is a warning):

screen shot 2017-02-15 at 00 43 03

I wonder what are the reasons for this. I think a missing ES6 export should totally be a compile error because it is so easy to mess it up, and then have weird runtime errors as a result.

I have two questions:

  • Why isn’t in an error by default?
  • Can we make this an error in CRA setup?
@gaearon gaearon added this to the 0.10.0 milestone Feb 15, 2017
@Timer
Copy link
Contributor

Timer commented Feb 15, 2017

Seems like we need to set strictModuleExceptionHandling to true.
It was added but never documented, and that's why we missed it.

@gaearon
Copy link
Contributor Author

gaearon commented Feb 15, 2017

Rad. Anybody wants to make a PR?

@insin
Copy link
Contributor

insin commented Feb 15, 2017

The config schema documentation for this says "Handles exceptions in module loading correctly at a performance cost" - should this be switched on in production builds?

@Timer
Copy link
Contributor

Timer commented Feb 15, 2017

Perhaps we can test it on a relatively large application? Would anyone be willing to do this?

@insin
Copy link
Contributor

insin commented Feb 15, 2017

It looks like this config triggers generation of try/catch wrappers when calling the generated module function at runtime, but doesn't upgrade missing modules to an error at build time.

@Timer
Copy link
Contributor

Timer commented Feb 15, 2017

After further investigation after what @insin, it seems strictModuleExceptionHandling isn't what we want (despite some various sources I found). It looks like we need to add a custom hook to parse the webpack warnings.

Maybe @sokra could give us some more insight?

@mqklin
Copy link

mqklin commented Feb 20, 2017

+1, doesn't work for me. I've added output: { strictModuleExceptionHandling: true } option in webpack config and import { nonexistentModule } from 'someFile' in a source file, no errors in terminal.

@sokra
Copy link

sokra commented Feb 20, 2017

strictModuleExceptionHandling is not related to import. It's about exceptions in modules. There is currently no way to make the warning an error (except with a custom plugin).

@gaearon
Copy link
Contributor Author

gaearon commented Feb 20, 2017

Is there a reason why? I’m just curious what the rationale is.

@sokra
Copy link

sokra commented Feb 20, 2017

Some typescript constructs emit incorrect import/exports (related to the loader impl, not sure if it's fixed now). So we used a warning. But I guess it fine to add an option upgrading it to an error.

@Timer
Copy link
Contributor

Timer commented Feb 22, 2017

See issue webpack/webpack#4347 and PR webpack/webpack#4348.

@Timer
Copy link
Contributor

Timer commented Feb 23, 2017

Once webpack/webpack#4348 is merged we will be able to set module.strictExportPresence to true for this behavior.

Is it worth setting strictModuleExceptionHandling to true? From what I understand it's more compliant with spec.

/cc @gaearon

@gaearon
Copy link
Contributor Author

gaearon commented Feb 23, 2017

I'd rather avoid a try catch in init code.

@Timer
Copy link
Contributor

Timer commented Feb 25, 2017

module.strictExportPresence was merged. Waiting for webpack release.

@Timer
Copy link
Contributor

Timer commented Mar 22, 2017

Released in WP 2.3.0.

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
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

5 participants