-
Notifications
You must be signed in to change notification settings - Fork 346
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
Add custom LOADER_CLASS support #210
Conversation
Would love to have this! |
This would also be huge for me too! |
Thanks @rhyneav. Sorry for the delay. The changes look good to me. I think we can ship this in 0.7.0. Can you please take a look at the CI failure and fix it if needed? |
No worries at all @owais! I've been in your shoes before maintaining an open source project. It looks like job Jobs Please let me know if you need me to dig into anything else! |
Thanks. I'll try to fix the test suite over the weekend and hopefully we can merge this next week. PRs welcome as always :) |
That sounds great! Thank you again for creating this project, it's been super helpful! |
Question from someone I think this might help: In prod my files are in a CDN. If I made a custom stats file that had CDN urls instead of local files, will this work? Or so I need to do an extra step to modify the template piece as well? Trying to upgrade from pipelines to this, and that is the last thing I gave to trace down. |
Yes, this should work already without the need for this PR. We do support having a custom path in different environments. |
Check the |
Amazing sorry to Hijack this PR.... this PR still looks good FWIW :)
…On Sat, Jan 11, 2020 at 8:33 AM Owais Lone ***@***.***> wrote:
Check the publicPath config option for webpack-bundle-tracker:
https://github.com/owais/webpack-bundle-tracker
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#210>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALZ2XZVM6736U4CYT3K5GDQ5HDDNANCNFSM4JGMM5AQ>
.
|
@rhyneav Migrated to CircleCI. Can you rebase this branch on master and push again please? |
c662252
to
166a352
Compare
Nice! CircleCI is great. I just rebased and pushed back up. Please let me know if you'd like me to add anything else! |
@rhyneav I think CI was configured to not build PRs from forks. Can you please --amend the commit and force push to trigger new builds? Thanks |
`LOADER_CLASS` on the `WEBPACK_CONFIG` setting is now where the loader class is defined. To retain backward compatibility and to keep getting started simple, this defaults to the existing WebpackLoader class. `WebpackLoader._load_assets` has been renamed to `WebpackLoader.load_assets`. This keeps the API extendable when creating custom webpack loaders. Documentation has been updated to include how to extend the WebpackLoader using the `LOADER_CLASS`.
166a352
to
5314fec
Compare
Just pushed! |
This project has been a huge value add for us, and I'd like to help address some of the issues around adding custom loader class support. It sounds like the approach outlined in #83 gives the most flexibility for extending the loader. #202 looks like it's pretty close to that implementation, but given that it's been almost three months since opening (without any edits), and the change is relatively small, I'd like to open a new PR in hopes of getting these changes upstream.
Changes:
LOADER_CLASS
on theWEBPACK_CONFIG
setting is now where the loader class is defined. To retain backward compatibility and to keep getting started simple, this defaults to the existingWebpackLoader
class.WebpackLoader._load_assets
has been renamed toWebpackLoader.load_assets
. This keeps the API extendable when creating custom webpack loadersDocumentation has been updated to include how to extend the WebpackLoader using the loader class.
Tests have also been added.
I'm happy to implement any feedback or discuss the design choices! I have the bandwidth.