-
Notifications
You must be signed in to change notification settings - Fork 23
feat(holocron-module-register-plugin): move to webpack 5 #154
Conversation
a2773c2
to
162287d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
@PixnBits The holocron package should not need a major version bump, only the webpack plugin |
packages/holocron-module-register-webpack-plugin/HolocronModuleRegisterPlugin.js
Outdated
Show resolved
Hide resolved
6455ee1
to
3d20f15
Compare
BREAKING CHANGE : This change is breaking as it directely interacts with webpacks source structure, which is changed in v5
1e3e691
to
b54c944
Compare
Co-authored-by: Matthew Mallimo <[email protected]>
@@ -130,6 +134,6 @@ describe('HolocronModuleRegisterPlugin', () => { | |||
|
|||
await waitForWebpack(options); | |||
const fileContents = fs.readFileSync(path.join(buildPath, outputFileName)).toString(); | |||
expect(fileContents.endsWith(`Holocron.registerModule("${moduleName}", ${holocronModuleName});})();`)).toBe(true); | |||
expect(fileContents).toContain('Holocron.registerModule("some-module", holocronModule-some-module);'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The kebab-case holocron-some-module
can't be right. Isn't that supposed to be a variable name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, however this tests shows that the plugin takes the name specified. We are specifying a non-camel-case module variable name up on Line 122
I have not changed this, just made the 'snapshot' literal instead of templated, so we can visually see that the name passed is accepted and inserted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in ea09da2
ea09da2
Description
Support webpack 5 in a new major version of the registration plugin
NOTE: this should be released as alpha until its consumed into one-app-bundler and rigorously tested.
How Has This Been Tested?
unit tests, used to build a module locally
Types of Changes
Checklist:
What is the Impact to Developers Using Holocron?
None, should be perfectly drop in