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

Don’t mess with webpack calculated dependencies #64

Closed
wants to merge 2 commits into from
Closed

Don’t mess with webpack calculated dependencies #64

wants to merge 2 commits into from

Conversation

diligiant
Copy link

This fixes this blocking (for us anyway !) webpack issue which wasn't webpack's fault.

When ProductionModePlugin rewrites the modules,

  1. It sets all but the new entry module (picked “randomly”, see below) to module.exports = __webpack_require__(${globalizeModuleIds[0]});* whereas some module might just be module.exports = {}; leading Wepback to consider that __webpack_require__ isn’t necessary, resulting in a module glue like function(module, exports) { (missing third parameter) which leads to a runtime crash.
  2. Like said above, the entry module being “randomly chosen”, it can be an empty module (module.exports = {};) overwritten with Globalize code leading to the same __webpack_require__ crash as in 1.

*As a side note, globalizeModuleIds[0] implies that 'globalize' is the first globalize entry in webpack.config chunk configuration. We should probably document that explicitely.

Signed-off-by: Frédéric Miserey frederic@none.net

When ProductionModePlugin rewrites the modules,
1. it sets all but the new entry module (picked “randomly”, see below) to `module.exports = __webpack_require__(${globalizeModuleIds[0]});` whereas some module might just be `module.exports = {};` leading Wepback to consider that __webpack_require__ isn’t necessary, resulting in a module glue like `function(module, exports) {` which leads to a runtime crash.
2. The entry module being “randomly chosen”, it can be an empty module (`module.exports = {};`) overwritten with Globalize code leading to the same __webpack_require__ crash as in 1.

Signed-off-by: Frédéric Miserey <frederic@none.net>
@jbellenger
Copy link
Collaborator

Hi @diligiant!
I'm not sure I understand what you mean by the entry module being randomly chosen. I believe the entry module should always be the globalize-compiled-data module that contains the formatters/parsers/etc for the locale. It should never be a Globalize proxy that does not define the formatters/parsers/etc.

I believe these guarantees are manifested here, though maybe there's a faulty assumption somewhere. Could you describe the randomness that you're seeing?

@diligiant
Copy link
Author

diligiant commented Jun 19, 2017

It is randomly chosen amongst the .tmp files.

I've had this problem since day 1: Some of the files in .tmp only have module.exports = {}; (for example as the result of just calling Globalize.locale() in one source file, no localization). When this is the case, WP2 "optimizes" the wrapper and doesn't include __webpack_require__ as its third parameter (check my WP issue).

It would be fine if we didn't mess with the files later on: the picked entry module can be one of those (m.e={}). Code needing __webpack_require__ is injected but WP2 doesn't know (it already calculated the "dependencies") and we end up (like I did - there is a sample project in the WP issue) with a crashing project.

Same with the non-entry modules: Some can be m.e={} and we inject code that explicitly uses __webpack_require__.

@rxaviers
Copy link
Owner

@jbellenger any input? Thanks

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

Successfully merging this pull request may close these issues.

webpackUniversalModuleDefinition wrapper missing __webpack_require__ parameter
3 participants