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

[Developer Experience] Enable hot-reloading in development #33

Merged
merged 1 commit into from Apr 27, 2023

Conversation

phacks
Copy link
Contributor

@phacks phacks commented Apr 26, 2023

Description

This PR enables hot-reloading in development, meaning one does not need to click the “Update” button in chrome://extensions after a code change to see the changes in action.

It uses the fabulous Web Extensions feature of Parcel 2.

How to test

  • Checkout this branch
  • Run yarn to install the @parcel/config-webextension dependency
  • Clean up existing artefacts to make sure the branch works on its own: rm -rf dist extension .parcel-cache
  • Run yarn start, and install the extension locally. Important: the folder to load is now dist, not extension. Make sure that everything works as usual.
  • Make a change to options.html while on the extension options page. The change should be visible right away, or after a page refresh.
  • Make a change to the widget while looking at a pull request. The change should be visible right away, or after a page refresh.
  • Run yarn build to build the artefact
  • Load the built artefact (under dist) and make sure that everthing works as usual.

@phacks phacks requested a review from delete-44 April 26, 2023 15:04
@@ -32,7 +32,10 @@
"128": "icons/icon128.png"
},
"web_accessible_resources": ["icons/*.png"],
"options_page": "src/options/options.html",
"content_security_policy": "script-src 'self' https://cdn.jsdelivr.net 'unsafe-eval'; object-src 'self'",
"options_ui": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options_ui page is functionally equivalent to options_page, but the latter isn’t recognized by Parcel.

"page": "options/options.html",
"open_in_tab": true
},
"content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need https://cdn.jsdelivr.net in the CSP anymore thanks to #30

@@ -7,8 +7,8 @@
"license": "MIT",
"private": true,
"scripts": {
"start": "mkdir -p extension && cp manifest.json extension/ && parcel src/github/background.js icons/* src/constants.js src/**/*.html src/**/*.css --dist-dir extension",
"build": "mkdir -p extension && cp manifest.json extension/ && parcel build src/github/background.js icons/* src/constants.js src/**/*.html src/**/*.css --dist-dir extension",
"start": "parcel watch src/manifest.json --config @parcel/config-webextension --host localhost --no-cache",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing --dist-dir is a bit of an awkward change, is there any reason you removed it? I've re-added it locally and everything still seems to work from /extension

Copy link
Contributor Author

@phacks phacks Apr 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question—I was wondering why it was specified in the first place, rather than using the default folder. I couldn’t remember (maybe it was necessary in Parcel v1? or I copied that from someplace?), so I thought it’d be slightly better to leave as much default settings as possible. In this specific example, if I’m new to this codebase a dist/ folder would look familiar (it’s where built files are stored in most JS projects), but an extension/ folder can be one of many things.
What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 Sounds good. It's better to make it closer to default now, whilst there's minimal dev interruption (ie, two people :P). If we do more work on this it would become more of a pain to switch out

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah definitely!

Copy link
Contributor

@delete-44 delete-44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooooo much nicer to work with 😍

@phacks phacks merged commit 57b1dd6 into main Apr 27, 2023
@delete-44 delete-44 deleted the dx/setup-hot-reloading branch May 2, 2023 08:48
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

2 participants