remove regex's, fix entry module #55
Conversation
- update regex to work with webpack2
- add tests cases for different module id scenarios - instead of importing globalize as module 1, get its module id from the webpack stats - move test output out of test dir, prevents endless loop when running `npm run test -- --watch` - lint tests
ProductionModePlugin.js
Outdated
| // - non-entry modules will be rewritten to export globalize | ||
| this.chunks | ||
| .filter((chunk) => /globalize-compiled-data/.test(chunk.name)) | ||
| .forEach((chunk) => { |
There was a problem hiding this comment.
Please, update package.json node engine accordingly for such support, i.e., arrow functions, const, etc.
There was a problem hiding this comment.
To reduce noise, I reverted all let/const/arrowfunctions. I'm happy to add them back in as a followup.
| var rimraf = require("rimraf"); | ||
| var fs = require("fs"); | ||
| var webpack = require("webpack"); |
There was a problem hiding this comment.
nitpick: let's sort this list :)
There was a problem hiding this comment.
Done. Added a rule for sort-requires to this PR
|
Thanks @jbellenger! Please, will that still work on webpack1? If it won't, let's simply remove the hacks to support both simultaneously and we tag it to lend as a next major. |
| "mocha": "^3.3.0", | ||
| "path-chunk-webpack-plugin": "^1.2.0", | ||
| "rimraf": "^2.6.1", | ||
| "webpack": "^1.15.0" | ||
| "webpack": "^2.5.1" |
ProductionModePlugin.js
Outdated
| newModule.context = module.context; | ||
| newModule.id = module.id; | ||
| newModule.dependencies = module.dependencies; | ||
| return newModule; |
| fnContent = globalizeCompilerHelper.compile(locale) | ||
| .replace("typeof define === \"function\" && define.amd", "false") | ||
| .replace(/require\("([^)]+)"\)/g, function(garbage, moduleName) { | ||
| return "__webpack_require__(" + globalizeModuleIdsMap[moduleName] + ")"; | ||
| }); | ||
|
|
||
| // Define all other individual globalize compiled data as a simple exports to Globalize. |
There was a problem hiding this comment.
Should we leave this comment? Otherwise, the else code below would be pretty confusing...
There was a problem hiding this comment.
Comment restored.
|
This looks great! |
|
Thanks for the comments, Rafael! I will:
I'll run through these and update this PR next week. |
|
webpack 1 dropped I think this could probably be fixed, but for the sake of simplicity I've just removed support for webpack1 and bumped the version to 0.4.0 It looks like the example project referenced in README.md might need to be updated. Production testing looks good As far as I know, this is ready to merge. |
- drop regexes - add support for string module ids - ensure that entry module is always executed - drop webpack 1 support - bump semver major Signed-off-by: James Bellenger <jbellenger@twitter.com>
jbellenger
force-pushed
the
entry-ordering
branch
from
ac465b5 to
e759154
Compare
|
Merged via 23b7dbe |
|
Thank you!! |
|
Released |
This is builds on PR 51: add support for string module ids. A simpler delta between this and PR 51 is here
Background
At twitter, we want to adopt webpack2 so that we can leverage the HashedModuleIds plugin. This will create stable hashes for our modules and chunks, improving caching for our users.
Problem
The present method of rendering localized chunks does not behave well when using string module ids and webpack2. ProductionModePlugin puts the globalize-compiled-data into the first module of a chunk, assuming that the first module is always executed as the entry module. This is not the case in webpack2, where the id of the entry module is an argument to webpackJsonp and rendered into the chunk.
Proposal
Replace the regex module rewriting that's done post-render with something more webpacky that can be done pre-render. This is essentially the same behavior as what is done currently, but using webpack primitives instead of strings. Using these webpack primitives, we can identify the chunks entry module and ensure that modules are rewritten correctly.
TODOs