aurelia / webpack-plugin

A plugin for webpack that enables bundling Aurelia applications.
MIT License
90 stars 36 forks source link

Correct support of Options.entry, use only first webpack entry point … #190

Closed josundt closed 3 years ago

josundt commented 3 years ago

This PR fixes a presumed unintentional change introduced in aurelia-webpack-plugin 5.0.0.

From the documentation of the entry property for the plugin options (link to Wiki page):

entry

entry: undefined | string | string[] = undefined

[...]

If you have multiple entry points, it adds them to the first one.

If you need to change that behavior, you can use this option to specify which entry point(s) AureliaPlugin should target. Note that this option expects an entry name, not a module name.

[...]

So when plugin options entry property is undefined, the aurelia plugin should be applied only to the first webpack entry point. When it is defined (string or string array), the aurelia plugin should be applied to the given webpack entry points only.

When upgrading to webpack 5, I found that this behavior had changed to something that was not in line with the documentation. When having more than one entry point in the webpack configuration, an error is thrown regardless of whether options.entry is defined or not. It looks like the entry property of the plugin options is ignored.

In our apps we normally have two webpack entry points, and we want the aurelia plugin to be applied only to one of them. That's how it worked in the webpack 4 version of the plugin. It worked when the webpack entry point for the aurelia app was the first, but to be explicit we also specified the entry point only in the plugin options entry property. Both things worked.

The implementation change in this PR should be more in line with the documentation and ensure better backward compatibility with the webpack 4 version of the plugin. The proposed change in this PR has been tested with our apps and fixes our issues.

This PR fixes the last of the issues we have found when migrating our applications to webpack 5. Hoping that the PR will be approved and hoping for a new release of this plugin plus aurelia-loader-webpack ASAP, so that we can finalize the migration! πŸ˜ƒ

Note: I did not build and commit changes to the /dist folder in this PR. Let me know if you also want me to build/commit/push these artifacts as well.

bigopon commented 3 years ago

@josundt thanks for this PR. I'll add some tests and take care of the dist part.

josundt commented 3 years ago

@bigopon Thanks πŸ‘. When do you plan to make the new releases of aurelia-loader-webpack and aurelia-webpack-plugin?

josundt commented 3 years ago

@bigopon Hi again. My company was hoping to migrate to webpack 5 before the upcoming product release. The migration is more or less complete, just waiting for this PR to be merged followed by new releases of aurelia-loader-webpack and aurelia-webpack-plugin. Can you give me hint about the time it will take on your end until we can expect this?

bigopon commented 3 years ago

@josundt hi there, thanks for the nudge. I'll review shortly, and do the appropriate enhancement for this PR then I'll work with @EisenbergEffect for a 5.0.1 release.

bigopon commented 3 years ago

@josundt this is actually more difficult to write tests for than I imagined. The fix looks good so I'll go ahead. Thanks @josundt for this PR.

josundt commented 3 years ago

Thanks @bigopon. Since I have contributed with a recent bugfix to aurelia-loader-webpack as well, I hope the package releases will be coordinated as follows:

  1. Release new version of aurelia-loader-webpack.
  2. In aurelia-webpack-plugin, update minimum version constraint (package.json) for the aurelia-loader-webpack dependency to the latest release (bullet point 1).
  3. Release new version of aurelia-webpack-plugin
bigopon commented 3 years ago

@josundt thanks for the help with the order. ping @EisenbergEffect , in case you need some more details on the release

bigopon commented 3 years ago

The release process has been done per the expected updates above, thanks for the list @josundt aurelia-webpack-plugin@5.0.1 is out together with aurelia-loader-webpack@2.2.3

josundt commented 3 years ago

@bigopon Thanks a lot πŸ™‚ I spent the day updating one of our apps, and everything seems to work πŸ‘