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

@types/chrome needs to be installed manually when using pnpm #1116

Open
5 tasks done
Tracked by #1085
2wheeh opened this issue Oct 27, 2024 · 10 comments
Open
5 tasks done
Tracked by #1085

@types/chrome needs to be installed manually when using pnpm #1116

2wheeh opened this issue Oct 27, 2024 · 10 comments
Labels
bug Something isn't working contribution welcome good first issue Want to contribute to WXT? This is a good place to start
Milestone

Comments

@2wheeh
Copy link

2wheeh commented Oct 27, 2024

Describe the bug

when using npm, TS infers browser namespace successfully:

image

but when using pnpm, it fails without installing @types/chrome manually:

image

we may need to improve docs or fix: this might be related to how pnpm handles devDependencies differently from npm.

Reproduction

just start fresh projects with pnpm following installation docs.

pnpm dlx wxt@latest init

Steps to reproduce

after installing, put your cursor on browser.runtime.* to see the inferred types.
for example, browser.runtime.id in entrypoints/background.ts.

System Info

System:
    OS: macOS 15.0.1
    CPU: (11) arm64 Apple M3 Pro
    Memory: 2.53 GB / 36.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.18.0 - ~/.nvm/versions/node/v20.18.0/bin/node
    npm: 10.8.2 - ~/.nvm/versions/node/v20.18.0/bin/npm
    pnpm: 9.9.0 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 130.0.6723.70
    Safari: 18.0.1
  npmPackages:
    wxt: ^0.19.11 => 0.19.13

Used Package Manager

pnpm

Validations

@2wheeh 2wheeh added the pending-triage Someone (usually a maintainer) needs to look into this to see if it's a bug label Oct 27, 2024
@1natsu172
Copy link
Contributor

Is the extensionApi in config set to 'chrome'? Also try restarting ts-server on your editor. ref: https://wxt.dev/guide/essentials/extension-apis.html#disabling-the-polyfill

Although you are still experiencing differences in behavior due to the package manager?

@2wheeh
Copy link
Author

2wheeh commented Oct 28, 2024

@1natsu172

Is the extensionApi in config set to 'chrome'? Also try restarting ts-server on your editor.

yes, it is set to 'chrome' as configured after scaffolding the project via pnpm dlx wxt@latest init currently.

Although you are still experiencing differences in behavior due to the package manager?

yes, there seems to be an issue where browser.runtime fails to load properly.

without @types/chrome on the app devDependency

image image

with @types/chrome on the app devDependency

image image

plus, even in npm (or with @types/chrome in pnpm), the type inference fails for the snippet below from the docs while it works correctly with chrome namespace:

image image image

if this isn't a bug I may be missing something important here - would you have any suggestions?

@2wheeh
Copy link
Author

2wheeh commented Oct 28, 2024

as per #784 (comment), it seems to need to install @types/chrome when using pnpm.

when browser is a value it infers well but still when it is a type, it shows TS error.

@1natsu172
Copy link
Contributor

1natsu172 commented Oct 28, 2024

@2wheeh Alright, there are two problems. I understood it too.

First, for some reason in the pnpm environment, the internal dependency @types/chrome can't be resolved and the typeof chrome is failing. The workaround to this problem is to include @types/chrome in the devDeps on the app side, as you already understood.

Here's the code inside.

runtime: WxtRuntime & Omit<(typeof chrome)['runtime'], 'getURL'>;


The second thing is that the interface as a type cannot be inferred from the browser as you say. This one is not a package-manager dependent problem. I think it's an implementation glitch in WXT.

The workaround over here is below, and either approach should be used.

import { Runtime } from "wxt/browser";

// This types from webextension-polyfill
function handler1(params: Runtime.MessageSender) {}

// This types from @types/chrome(need install `@types/chrome`)
function handler2(params: chrome.runtime.MessageSender) {}

@2wheeh
Copy link
Author

2wheeh commented Oct 28, 2024

@1natsu172 thank you for the very clear summary 👍🏻

@aklinker1 aklinker1 added bug Something isn't working and removed pending-triage Someone (usually a maintainer) needs to look into this to see if it's a bug labels Nov 14, 2024
@aklinker1 aklinker1 added this to the v1.0 milestone Nov 16, 2024
@aklinker1
Copy link
Collaborator

Alright, so @types/chrome can't be resolved because when it's a subdependency, it's located at node_modules/.pnpm/@types/chrome. However, the triple-slash reference WXT adds (/// <reference types="@types/chrome" />) only looks for node_modules/@types/chrome, it doesn't look inside .pnpm directory, so it doesn't add the types correctly.

So the solution is to use import.resolve to find the true file path of @types/chrome, and use that in the reference instead of just "@types/chrome" here:

entries.push({ module: '@types/chrome' });

@aklinker1 aklinker1 added contribution welcome good first issue Want to contribute to WXT? This is a good place to start labels Nov 25, 2024
@aklinker1
Copy link
Collaborator

aklinker1 commented Nov 25, 2024

I'll try and get to this soon, but if someone else wants to give it a go, see my previous comment.

@1natsu172
Copy link
Contributor

Another issue is more complex, so I have split it into a separate issue. #1213

@aklinker1 aklinker1 mentioned this issue Nov 26, 2024
21 tasks
@2wheeh
Copy link
Author

2wheeh commented Nov 27, 2024

So the solution is to use import.resolve to find the true file path of @types/chrome, and use that in the reference instead of just "@types/chrome" here:

So getMainDeclarationEntry needs to be updated to resolve correct module path.

 if ('module' in ref) {
      return lines.push(`/// <reference types="${ref.module}" />`);
 }

but the thing is it seems not possible to use import.meta.* here...

ERROR Cannot use 'import.meta' outside a module :/

something around jiti or c12 might be related?

@aklinker1
Copy link
Collaborator

aklinker1 commented Nov 28, 2024

@2wheeh If you base off 0.20.0-breaking-changes, I've already merged the fix this issue. If you open a PR into that branch, it will go out in the next major version.

But we already have a method for finding the path to a NPM package in a CJS/ESM environment without import.meta:

async function resolveWxtModuleDir() {
// TODO: Use this once we're fully running in ESM, see https://github.com/wxt-dev/wxt/issues/277
// const url = import.meta.resolve('wxt', import.meta.url);
// resolve() returns the "wxt/dist/index.mjs" file, not the package's root
// directory, which we want to return from this function.
// return path.resolve(fileURLToPath(url), '../..');
const requireResolve =
globalThis.require?.resolve ??
(await import('node:module')).default.createRequire(import.meta.url)
.resolve;
// resolve() returns the "wxt/dist/index.mjs" file, not the package's root
// directory, which we want to return from this function.
return path.resolve(requireResolve('wxt'), '../..');
}

Forgot to link this. You could refactor this function to work with any NPM package. Then we could get this fix out before 0.20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working contribution welcome good first issue Want to contribute to WXT? This is a good place to start
Projects
None yet
Development

No branches or pull requests

3 participants