crxjs / chrome-extension-tools

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

`default_icon` in `manifest.action` does not support string #389

Closed Chersquwn closed 1 year ago

Chersquwn commented 2 years ago

Build tool

Vite

Where do you see the problem?

Describe the bug

It seems default_icon in action does not support string. If default_icon uses string, it will be split into 'i','c','o','n','s','/'..., such as icons/.

https://github.com/crxjs/chrome-extension-tools/blob/main/packages/vite-plugin/src/node/helpers.ts#L71

image

Reproduction

// manifest.json
{
  "name": "Demo",
  "version": "1.0.0",
  "manifest_version": 3,
  "action": {
    "default_title": "Demo Title",
    "default_icon": "icons/logo-128.png"
  },
  "icons": {
    "16": "icons/logo-16.png",
    "48": "icons/logo-48.png",
    "128": "icons/logo-128.png"
  }
}

Logs

[crx:manifest-post] ENOENT: Could not load manifest asset "i".
Manifest assets must exist in one of these directories:

System Info

System:
    OS: macOS 12.3.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 80.28 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.19.0 - ~/.nvm/versions/node/v14.19.0/bin/node
    Yarn: 1.22.18 - /usr/local/bin/yarn
    npm: 6.14.16 - ~/.nvm/versions/node/v14.19.0/bin/npm
  Browsers:
    Chrome: 102.0.5005.61
    Safari: 15.4
  npmPackages:
    rollup-plugin-chrome-extension: 4.0.1-25 => 4.0.1-25 
    vite: ^2.9.9 => 2.9.9

Severity

annoyance

filipw01 commented 2 years ago

@Chersquwn string for default icon is not supported in manifest v3, see https://json.schemastore.org/chrome-manifest. @jacksteamdev I guess the manifest validation could pick it up though right?

Chersquwn commented 2 years ago

@filipw01 But I don't see any migration guide about "default icon not support string type" on the chrome developer website.In the official examples, there are also default icons that use string. Like this,

image

and this, https://github.com/GoogleChrome/chrome-extensions-samples/blob/main/examples/bookmarks/manifest.json

filipw01 commented 2 years ago

In that case, the example has an error. action is a new thing and it's specified in a migration guide https://developer.chrome.com/docs/extensions/mv3/intro/mv3-migration/#action-api-unification

It is specified only as an object here https://developer.chrome.com/docs/extensions/reference/action/

This key is a dictionary of sizes to image paths

jacksteamdev commented 2 years ago

@Chersquwn Thanks for reporting this discrepancy between the Chrome developer docs and CRXJS! I downloaded and tested the sample extension with default_icon as a string and it does work in chromium@v102.

I'm open to adding support for icons as strings. PRs are welcome!

What @filipw01 said is best practice. Chrome uses different icon sizes based on the user's screen resolution, and a low-res icon looks unprofessional. I don't know what happens to a high-res icon in a lower res setting.

Notes

I'm unsure what the Chrome Web Store would do with an extension like this. Would it pass the review?

The JSON schema is great, it provides IntelliSense as well as validation. I've added validation to the roadmap.

VSCode instructions for using JSON schemas: https://code.visualstudio.com/docs/languages/json#_json-schemas-and-settings