bundle-loader
bundle-loader copied to clipboard
webpack-contrib
feat: add `require.ensure` error callback
Adds support for an optional error callback to the bundle-loader.
This work was originally done by @richardscarrott: https://github.com/richardscarrott/require-error-handler-webpack-plugin/blob/master/src/BundleLoader.js.
This PR uses the version created by @richardscarrott with refactoring to reflect changes from the bundle-loader repo since the fork was created.
Agreed about the tests. Too bad there weren't any tests to begin with...
The require.ensure with error handling does have tests though, so I think it would be sufficient to just test that the correct syntax of require.ensure is called by bundle-loader (for the various permutations of with/without error callback, and with/without chunk name.)
Are there any webpack loaders with tests like that that I could use as a reference for adding some tests to bundle-loader?
Also, what is webpack-defaults?
Also, what is webpack-defaults?
It's a default infrastructure we use for loaders and packages. It comes with opinionated testing setup etc. and allows us to maintain the setup as a dependency (it syncs and migrates project config).
Should be done after defaults, i'll get the pull request in this evening.
@jharris4 - https://github.com/webpack-contrib/bundle-loader/pull/51 is webpack defaults
@d3viant0ne Ok, I'm still a little unsure as to how to proceed with creating tests for this package, if that's a hard requirement for getting my PR merged...
If it helps put minds at east, I have been using this code in production for months now with no issues via a fork of bundle-loader which is identical to this PR: https://github.com/jharris4/split-chunk-loader
First and foremost, the pull request for defaults has to land in master which will lay the foundations to actually author tests and landing this with defaults is more about not releasing back to back Major version releases than anything else.
As far as tests for this specific pull request, we need to add tests to cover the existing functionality. If those don't provide a path forward, we can deal with it then.
Any word on the defaults? Anything I can do to help move this forward and get it merged?
@jharris4 Should be on the finishing line, sry for the inconvenience
Hope the next release will coming soon! Currently I just edit line.27 to handle loading errors.
" }, function(err) { cb(null, err); }, " + chunkNameParam + ");\n",
@d3viant0ne Any updates on the status of this? Anything I can do to try and help speed things along?
@jharris4 - It's technically done. I'll pick up the latest version of defaults & get the other org maintainers to give it a final once over before it merges.
@d3viant0ne Still curious to see if this is gonna move forward :-)
If it helps to get things moving I can try and do some work to integrate webpack-defaults into the fork of this repo that i'm using (https://github.com/jharris4/split-chunk-loader) to make sure everything works. But I could use some pointers on how to do that... :-)
@michael-ciniawsky I made the changes you requested. Dare I hope that you guys might be close to actually merging this? :-)
@michael-ciniawsky @d3viant0ne
Hi guys, so it has been 11 months since I created this PR! Is there anything I can do to move this forward?
I'm starting to look at upgrading to webpack 4 and hopefully this loader won't require any changes for that...
@michael-ciniawsky @d3viant0ne I just merged the changes from version 0.5.6 so the build should be passing again soon.
@michael-ciniawsky @d3viant0ne Or not! No idea why the travis builds are failing... Something about the rake command. The command "rake" exited with 1.
@michael-ciniawsky @d3viant0ne I just fixed a small issue with the error callback not getting passed the error object in non-lazy mode.
We've been using this in production for months now. Any chance we can get this merged so that I can stop maintaining https://github.com/jharris4/split-chunk-loader ? :-)
I'm using the code from this PR and it's working well. Is there any hope with getting this merged?
