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

Add support for chrome-extension:// protocol to bundle-url.js #2434

Merged
merged 2 commits into from Dec 21, 2018

Conversation

rhurstdialpad
Copy link
Contributor

↪️ Pull Request

This PR adds support for dynamic bundle loading (with code splitting) for the chrome-extension:// protocol.

Usually, it's not necessary to use code-splitting with a chrome extension as the resources are loaded from disk. That said if you build your application for the web as well as a chrome extension, attempting to keep a unified code base and using code splitting to optimize the web build, you'll be forced to use code splitting for your chrome extension too.

This shouldn't be a problem; however, parcels logic for detecting bundle paths uses a regex that includes supported protocols. file://, http://, https:// and ftp:// are present, but not chrome-extension:// which causes it to always load bundles relative to the root path when using chrome-extension://.

@macklinu
Copy link

What would an integration test look for this change? I'm honestly not sure, just curious about how this could be tested with an integration test.

@rhurstdialpad
Copy link
Contributor Author

rhurstdialpad commented Dec 19, 2018

Not sure. @DeMoorJasper @devongovett Any suggestions?

@DeMoorJasper
Copy link
Member

@rhurstdialpad For this change it would probably be good enough to just add some unit tests, importing src/builtins/bundle-url.js and using the exported getBundleURL and just put a couple urls through it and see if it returns the expected result.

We however do need some better testing for async bundle loading in the near future to prevent future issues. You can possibly improve our browser sandbox to also support bundle loading for integration tests. See https://github.com/parcel-bundler/parcel/blob/master/packages/core/parcel-bundler/test/utils.js#L64

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

3 participants