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

Upgrade to Manifest V3 on Chrome #166

Closed
wants to merge 2 commits into from
Closed

Conversation

Asymons
Copy link
Contributor

@Asymons Asymons commented Jan 4, 2025

Description

Support for Manifest V2 is coming close to an end (seems to be June 2025) so in order to maintain this project, we need to upgrade to Manifest V3.

This also removes the warning from the Chrome Extensions Web Store.
image

The changes are quite simple and deviate slightly from the implementation in react-devtools. I originally thought to follow what was done there but quickly came to understand I was introducing a bunch of unnecessary code and abstraction that may not be needed for this project.

The upgrade to Manifest V3 from V2 requires a few changes, primarily related to security, so here's what we did:

  1. Remove unsafe inline execution of scripts. In order to run content scripts, we need to declare them explicitly in the manifest.
  2. Use action instead of browserAction to set icons & popups. This is just a breaking API interface change but doesn't impact functionality.
  3. Update the manifest to support V3.

Tests

I basically ran everything on Chrome as it seems Firefox isn't supported (had some trouble getting it running personally). I ran our light unit tests yarn test and the video below demonstrates me poking around a website that supports Relay.

Screen.Recording.2025-01-03.at.9.35.42.PM.mov

Copy link
Contributor

@tyao1 tyao1 left a comment

Choose a reason for hiding this comment

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

Amazing! Thanks for upgrading it. The changes are so clean.

@Asymons
Copy link
Contributor Author

Asymons commented Jan 6, 2025

@tyao1 seems like the pipeline is broken (related to a bad node version), have we encountered this before or is this new?

@facebook-github-bot
Copy link
Contributor

@tyao1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@tyao1
Copy link
Contributor

tyao1 commented Jan 7, 2025

@tyao1 seems like the pipeline is broken (related to a bad node version), have we encountered this before or is this new?

It is pre-existing. It was probably broken by 7edce8e

@Asymons
Copy link
Contributor Author

Asymons commented Jan 13, 2025

@tyao1 just following up with this - is someone assigned to fix the pipeline already? don't want to delay this PR too much to get this back on the chrome store (I think some users were facing issues downloading it actually due to the warning)

NVM my bad, I didnt see that you've already updated the node version :D

@facebook-github-bot
Copy link
Contributor

@Asymons has updated the pull request. You must reimport the pull request before landing.

@Asymons Asymons requested a review from tyao1 January 13, 2025 02:11
@facebook-github-bot
Copy link
Contributor

@tyao1 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@tyao1
Copy link
Contributor

tyao1 commented Jan 16, 2025

@Asymons I tried it locally and built the unpacked version with yarn build:extension:chrome:dev, but it throws the error and didn't work
image

Did you get the same error?

@tomgasson
Copy link

This works fine for me when build for production and run as an unpacked extension with yarn build:extension:chrome

I can also reproduce the problem @tyao1 has run into - when using yarn build:extension:chrome:dev (note the :dev) it's outputting in webpack's eval() sourcemap format which is explicitly not supported in Manifest V3, which should be resolvable by switching webpack's devtool

@Asymons
Copy link
Contributor Author

Asymons commented Jan 31, 2025

@tomgasson thank you for investigating this 🙏 @tyao1 apologies this slipped under the radar for me.

would we be fine with merging this in and doing a follow up PR to get dev working again? not sure what the implications are for switching to webpack's devtool. I understand that this may not be ideal given we're merging in something that'll break the dev build; however, I guess it's better than this extension being effectively offline 😅 I'll leave it up to you as one of the core maintainers to make the call

@tyao1
Copy link
Contributor

tyao1 commented Feb 1, 2025

I will let the team review the merge and publish the new version once it's merged

@facebook-github-bot
Copy link
Contributor

@tyao1 merged this pull request in 64111b8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants