-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Include declarativeNetRequest JSON file rules as dependencies when bundling web extensions #8189
Conversation
@101arrowz I believe you are the Parcel maintainer that has worked on web extensions most recently. Will you look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, just a few suggestions.
packages/transformers/webextension/src/WebExtensionTransformer.js
Outdated
Show resolved
Hide resolved
packages/transformers/webextension/src/WebExtensionTransformer.js
Outdated
Show resolved
Hide resolved
packages/transformers/webextension/src/WebExtensionTransformer.js
Outdated
Show resolved
Hide resolved
Also make sure to fix flow issues (probably just by typecasting). |
@101arrowz I've made those changes, including fixing the flow typing issues. Also, I don't think the test failures (at least on the previous run) are due to anything I've change.
|
Would be nice to include this in the integration test: https:/parcel-bundler/parcel/tree/v2/packages/core/integration-tests/test/integration/webextension
Yes, the tests are flakey... don't worry about them. |
@mischnic I can look into that today or tomorrow. |
I've modified |
That should be working (and there's no need to run a build step, Babel register does this on the fly). (You need to modify |
The asset bundler already has error handling for missing files.
A passing integration test has been added. I messed up the git history of this branch by rebasing off of |
packages/transformers/webextension/src/WebExtensionTransformer.js
Outdated
Show resolved
Hide resolved
packages/transformers/webextension/src/WebExtensionTransformer.js
Outdated
Show resolved
Hide resolved
Think this should be good to go. Thanks for working on this! |
↪️ Pull Request
Fixes issue #8188.
I have included the
declarative_net_request
property in themanifest.json
schema validator. I have also written a portion in@parcel/transformer-webextension
that adds JSON files specified in thepath
property as dependencies of the asset, complete with error message if the specified JSON file does not exist.This is my first contribution to Parcel, so please let me know what I need to change to align with correct API usage, design, and coding standards.
💻 Examples
See my example repo. Clone it, run
npm install
, and thennpm run build
to build the extension. If you checkdist/
, there is norules.json
file in it. If you loaddist/
as an unpacked extension into Chrome, it will throw an error about howrules.json
is missing.🚨 Test instructions
Install my branch into my example repository, then build the extension again. You will see that
rules.json
has been copied into thedist/
folder.✔️ PR Todo