crxjs / chrome-extension-tools

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

Error during build: crx:content-script-resources #425

Closed MichaelVidStep closed 2 years ago

MichaelVidStep commented 2 years ago

Build tool

Vite

Where do you see the problem?

Describe the bug

When I try to build my extension I get an error. I've tried removing several packages and modifying my manifest file with no luck so far.

Reproduction

Logs

[crx:content-script-resources] TypeError: Cannot read properties of undefined (reading 'split')
    at stubMatchPattern (C:\Programming Projects\vidstep\node_modules\@crxjs\vite-plugin\dist\index.cjs:137:35)
    at Array.map (<anonymous>)
    at Object.renderCrxManifest (C:\Programming Projects\vidstep\node_modules\@crxjs\vite-plugin\dist\index.cjs:3355:59)
    at Object.generateBundle (C:\Programming Projects\vidstep\node_modules\@crxjs\vite-plugin\dist\index.cjs:2952:60)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Bundle.generate (C:\Programming Projects\vidstep\node_modules\rollup\dist\shared\rollup.js:16114:9)
    at async C:\Programming Projects\vidstep\node_modules\rollup\dist\shared\rollup.js:23730:27
    at async catchUnfinishedHookActions (C:\Programming Projects\vidstep\node_modules\rollup\dist\shared\rollup.js:23172:20)
    at async doBuild (C:\Programming Projects\vidstep\node_modules\vite\dist\node\chunks\dep-8f5c9290.js:41701:20)
    at async build (C:\Programming Projects\vidstep\node_modules\vite\dist\node\chunks\dep-8f5c9290.js:41527:16)
[crx:manifest-post] Error in crx:content-script-resources.renderCrxManifest
error during build:
Error: Error in crx:content-script-resources.renderCrxManifest
    at Object.generateBundle (C:\Programming Projects\vidstep\node_modules\@crxjs\vite-plugin\dist\index.cjs:2963:19)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Bundle.generate (C:\Programming Projects\vidstep\node_modules\rollup\dist\shared\rollup.js:16114:9)
    at async C:\Programming Projects\vidstep\node_modules\rollup\dist\shared\rollup.js:23730:27
    at async catchUnfinishedHookActions (C:\Programming Projects\vidstep\node_modules\rollup\dist\shared\rollup.js:23172:20)
    at async doBuild (C:\Programming Projects\vidstep\node_modules\vite\dist\node\chunks\dep-8f5c9290.js:41701:20)
    at async build (C:\Programming Projects\vidstep\node_modules\vite\dist\node\chunks\dep-8f5c9290.js:41527:16)
    at async CAC.<anonymous> (C:\Programming Projects\vidstep\node_modules\vite\dist\node\cli.js:738:9)
error Command failed with exit code 1.

System Info

System:
    OS: Windows 10 10.0.19044
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
    Memory: 2.76 GB / 15.95 GB
  Binaries:
    Node: 16.13.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.18 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.5.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 102.0.5005.63
    Edge: Spartan (44.19041.1266.0), Chromium (102.0.1245.39)
    Internet Explorer: 11.0.19041.1566

Severity

annoyance

thetarnav commented 2 years ago

I have the same error after updating @crxjs/vite-plugin. The affected versions seem to be 1.0.71.0.11. I'm on Win 10 too.

jacksteamdev commented 2 years ago

@thetarnav @MichaelVidStep Could you provide the manifest.json that causes the error? I'm especially interested in content script match patterns:

{
  "content_scripts": [{
    "matches": ["https://*.google.com/*"] // 👈 this array is what i need
  }]
}

Notes

This error happens when the content script plugin tries to convert a match pattern from a content script into a match pattern for a web-accessible resource:

https://github.com/crxjs/chrome-extension-tools/blob/454195826e265d4f62525a861b88a7174f85dae1/packages/vite-plugin/src/node/plugin-content-scripts.ts#L529-L533

Then the error happens here:

https://github.com/crxjs/chrome-extension-tools/blob/454195826e265d4f62525a861b88a7174f85dae1/packages/vite-plugin/src/node/helpers.ts#L164-L169

I think I can refactor this so we get better errors and the code is easier to read.

thetarnav commented 2 years ago

Could you provide the manifest.json that causes the error?

https://github.com/thetarnav/solid-devtools/blob/main/packages/extension/manifest.ts I won't be surprised if it's full of errors — don't know what I'm doing really. I was copying vue-devtools I think.

jacksteamdev commented 2 years ago

@thetarnav Thanks for the quick repro! :pray: I'm pretty sure <all_urls> triggers the bug. We can fix that.

https://github.com/thetarnav/solid-devtools/blob/b576605dccfa698776d7502d6fb521e5b396f6ea/packages/extension/manifest.ts#L11-L17

Best Practice

Lots of devs use <all_urls>, but Google kind of frowns on it; I've heard that it triggers a more thorough review when you publish it on the Chrome Web Store because it also runs on the file: scheme.

I suggest switching to *://*/* unless you really need to match the file: scheme. Even then, I would use explicit match patterns. That makes the CWS review process more straightforward b/c they can tell you know what you're doing.

{
  "content_scripts": [
    {
      "matches": [
        "http://*/*",
        "https://*/*",
        "file:///*"
      ],
      "js": [
        "content/content.ts"
      ],
      "run_at": "document_start"
    }
  ]
}

Notes

This is something I'll have to check if we can support. I wonder if web_accessible_resources supports <all_urls>? Maybe we can map it to explicit match patterns.

thetarnav commented 2 years ago

Thank you, changing the pattern to *://*/* seems to work just fine with the latest version.

As for the <all_urls>, it probably would be better to support it, but give some sort of friendly warning about it. Just because the internet is filled with this pattern mentioned as an option.

MichaelVidStep commented 2 years ago

@thetarnav Thanks for the quick repro! 🙏 I'm pretty sure <all_urls> triggers the bug. We can fix that.

https://github.com/thetarnav/solid-devtools/blob/b576605dccfa698776d7502d6fb521e5b396f6ea/packages/extension/manifest.ts#L11-L17

Best Practice

Lots of devs use <all_urls>, but Google kind of frowns on it; I've heard that it triggers a more thorough review when you publish it on the Chrome Web Store because it also runs on the file: scheme.

I suggest switching to *://*/* unless you really need to match the file: scheme. Even then, I would use explicit match patterns. That makes the CWS review process more straightforward b/c they can tell you know what you're doing.

{
  "content_scripts": [
    {
      "matches": [
        "http://*/*",
        "https://*/*",
        "file:///*"
      ],
      "js": [
        "content/content.ts"
      ],
      "run_at": "document_start"
    }
  ]
}

Notes

This is something I'll have to check if we can support. I wonder if web_accessible_resources supports <all_urls>? Maybe we can map it to explicit match patterns.

Yep this solved my problem. Thanks so much!