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

Fix support for DynamicEntryPlugin #1319

Merged
merged 3 commits into from Feb 25, 2018

Conversation

mikegreiling
Copy link
Member

@mikegreiling mikegreiling commented Feb 25, 2018

  • This is a bugfix
  • This is a code refactor
  • This is a test update
  • This is a typo fix
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes. I have added tests for this change and I have added new tests for prior behavior to prove that this doesn't break anything.

Motivation / Use-Case

The support for DynamicEntryPlugin added in #802 was incorrect. This fixes addDevServerEntrypoints.js as it applies to dynamic endpoints (functions which return an entry object and functions which return a Promise)

Breaking Changes

none.

Additional Info

This pull request targets the webpack-4 v3.x branch, but I would like to port this back to 2.x as well if you plan to make any more releases under that major version. The only conflict would be the default entry point added in #1310. I could simply exclude that behavior from this change and open up a separate PR targeting the master branch.

Closes: #1318

wpOpt.entry[key] = devClient.concat(wpOpt.entry[key]);
const prependDevClient = (entry) => {
if (typeof entry === 'function') {
return () => Promise.resolve(entry()).then(prependDevClient);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will handle both functions which return entry objects and functions which return a Promise due to the behavior of Promise.resolve. It is the same technique used in the DynamicEntryPlugin itself (link).

const webpackOptions = {};
const devServerOptions = {};

addDevServerEntrypoints(webpackOptions, devServerOptions);

assert(webpackOptions.entry.length, 2);
assert(webpackOptions.entry[1], './src');
assert.equal(webpackOptions.entry.length, 2);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed this test also. By using assert and not assert.equal, these tests were not doing anything other than ensuring webpackOptions.entry.length was not falsey.

@SpaceK33z SpaceK33z merged commit f4f14ce into webpack:webpack-4 Feb 25, 2018
@SpaceK33z
Copy link
Member

Thanks for this fine PR!!

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.

None yet

2 participants