crxjs / chrome-extension-tools

Bundling Chrome Extensions can be pretty complex. It doesn't have to be.
https://crxjs.dev/vite-plugin
2.98k stars 192 forks source link

Strip paths for `web_accessible_resources`'s `matches` #282

Closed riywo closed 2 years ago

riywo commented 2 years ago

Describe the problem

When creating an internal chrome extension, we sometime specify *//*/*foo.json* for matches of content_scripts. This works well when developing because RPCE only adds <all_urls> for web_accessible_resources's matches.

However, when building production manifest, RPCE just copies matches from content_scripts. This is an issue because Manifest V3 doesn't allow *://*/*foo.json* for web_accessible_resources's matches:

Only the origin is used to match URLs. Origins include subdomain matching. Paths are ignored. https://developer.chrome.com/docs/extensions/mv3/manifest/web_accessible_resources/#manifest-declaration

Therefore, we can't load the production manifest at all.

Invalid value for 'web_accessible_resources[0]'. Invalid match pattern.
Could not load manifest.

Describe the proposed solution

The problem here is path. Chrome accepts the manifest if I strip the path like *://*/*.

I believe we can strip the path part around here https://github.com/extend-chrome/rollup-plugin-chrome-extension/blob/a0d0c0eed17100083f74dbb270ad3c42ccc82d41/src/node/plugin-content-scripts.ts#L510

Alternatives considered

We can specify only domains in content_scripts's matches but it can be applied to broader paths which we might not want.

A workaround script after npm run build:

const manifest = require('../dist/manifest.json')
const fs = require('fs')

const re = /^(?<schema>[^:]+):\/\/(?<domain>[^\/]+)(?<path>\/.+)$/;

for (const web_accessible_resource of manifest.web_accessible_resources) {
  const revisedMatches = web_accessible_resource.matches.map(pattern => {
    const match = re.exec(pattern)
    if (match?.groups) {
      const { schema, domain } = match.groups
      return `${schema}://${domain}/*`
    } else {
      return pattern
    }
  })
  web_accessible_resource.matches = revisedMatches
}

fs.writeFileSync('dist/manifest.json', JSON.stringify(manifest, null, 2))

Importance

nice to have

jacksteamdev commented 2 years ago

@riywo Good idea, we should sanitize web_accessible_resources match patterns. :+1:

Would you like to make a PR?

riywo commented 2 years ago

Sure, I'll be able to send a PR in a week or so.

riywo commented 2 years ago

@jacksteamdev I'm trying to create an e2e test first, but npx jest tests/e2e on HEAD (i.e. without any modification) doesn't succeed on my macOS. Is there any caveat to run e2e tests?

jacksteamdev commented 2 years ago

@riywo Not that I know of. What error are you getting?

riywo commented 2 years ago

@jacksteamdev Hmm... I face multiple failures like this https://gist.github.com/riywo/c33d76b3e4abce574cfaaa94dcd2359d

JhaAman commented 2 years ago

I'm running into this issue and I have no idea how to really fix it like you guys seem to, so my production build simply doesn't work. My content matches looks like: "matches": ["http://localhost:8080/", "https://calendar.google.com/*"] and my web_accessible matches looks like "matches": ["<all_urls>"].

Is there any way to use the specified web_accessible matches or get path working in my project?

JhaAman commented 2 years ago

Until this is fixed, you can copy/paste the web_accessible_resources from your yarn dev build, and when you run yarn build you manually replace web_accessible_resources with whatever you copied from the dev build.

Or, I imagine OP's script automates that process.

jacksteamdev commented 2 years ago

@riywo @JhaAman This hit NPM in @crxjs/vite-plugin@1.0.7 :rocket:

Please let me know if it works for your use case! :pray:

yudai-nkt commented 2 years ago

I ran into the same problem and subscribed to this issues just a few hours before you closed this via PR. It seems working well for me at least!