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

shamefully-hoist breaks package isolation in a monorepo #286

Closed
rsweeneydev opened this issue Dec 15, 2023 · 9 comments
Closed

shamefully-hoist breaks package isolation in a monorepo #286

rsweeneydev opened this issue Dec 15, 2023 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@rsweeneydev
Copy link

rsweeneydev commented Dec 15, 2023

Describe the bug

wxt adds shamefully-hoist=true to an .npmrc file in the folder of the wxt extension. However, this setting changes hoisting for the entire monorepo. After running pnpm install, pnpm shows the following warning:

Scope: all 15 workspace projects
✔ The modules directory at "/Users/robert/code/ditto/node_modules" will be removed and reinstalled from scratch. Proceed? (Y/n) · true

Now the node_modules at the root of the monorepo has all modules from all packages in the monorepo hoisted. This breaks one of the main benefits of using pnpm.

To Reproduce

Steps to reproduce the behavior:

  1. mkdir bug-repro && cd bug-repro
  2. npm create svelte@latest my-app
  3. pnpx wxt@latest init my-ext
  4. Create a pnpm-workspace.yaml with my-app and my-ext in them
  5. npm install

Expected behavior

node_modules in root shouldn't have modules from my-app and my-ext

Environment

  System:
    OS: macOS 14.1.2
    CPU: (10) arm64 Apple M1 Max
    Memory: 15.90 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 9.6.7 - ~/.nvm/versions/node/v18.17.1/bin/npm
    pnpm: 8.12.0 - ~/.nvm/versions/node/v18.17.1/bin/pnpm
    bun: 1.0.15 - ~/.bun/bin/bun
  Browsers:
    Chrome: 120.0.6099.109
    Safari: 17.1.2
  npmPackages:
    wxt: ^0.12.0 => 0.12.0 

Additional context

I love wxt. It's incredible!

@rsweeneydev rsweeneydev added the pending-triage Someone (usually a maintainer) needs to look into this to see if it's a bug label Dec 15, 2023
@aklinker1
Copy link
Collaborator

aklinker1 commented Dec 15, 2023

I also use WXT in a pnpm monorepo, but we have an electron app that requires shamefully-hoist=true, so this isn't something I've thought about much. I created this project based off Nuxt, which, at the time, also required setting this flag. Now though, it doesn't seem like the case (nuxt/nuxt#14146).

So I can look into fixing this in WXT as well. Right now, there are 2 dependencies that prevent it from working:

  • webextension-polyfill
  • @webext-core/match-patterns

Unfortunately, I'm not an expert on PNPM and module resolution, so I don't know why only these two packages cause problems.

But if you list those two dependencies in your package.json, you can remove the .npmrc file. You can see this in a non-monorepo project:

$ pnpx wxt@latest init wxt-without-hoisting
$ cd wxt-without-hoisting
$ rm .npmrc
$ pnpm i
$ pnpm build
...
[vite]: Rollup failed to resolve import "webextension-polyfill" from "virtual:wxt-background?/Users/aklinker1/Development/local/wxt-without-hoisting/entrypoints/background.ts".
...

$ pnpm i webextension-polyfill
$ pnpm build
...
[vite]: Rollup failed to resolve import "@webext-core/match-patterns" from "virtual:wxt-background?/Users/aklinker1/Development/local/wxt-without-hoisting/entrypoints/background.ts".
...

$ pnpm i @webext-core/match-patterns
$ pnpm build
...
 Built extension

Same errors for dev mode.

So I don't really know where to start with this...

Nuxt solved their problems by listing vue and vue-router in their package.json. I will probably do this with webextension-polyfill, but I don't know why @webext-core/match-patterns is causing any problems...

@aklinker1
Copy link
Collaborator

Here's an issue for vite that talks more about it: vitejs/vite#324

And here's a issue that this was fixed in: preactjs/prefresh#75

Both talk about talk about packages importing a subdependency not listed in the library's package.json, but I do list @webext-core/match-patterns inside WXT's package.json, and it has 0 dependencies, so it's not importing any sub-dependencies.

@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 Dec 16, 2023
@aklinker1 aklinker1 self-assigned this Dec 16, 2023
@aklinker1
Copy link
Collaborator

aklinker1 commented Dec 16, 2023

Actually, I know what the problem is now.

[vite]: Rollup failed to resolve import "@webext-core/match-patterns" from "virtual:wxt-background?/Users/aklinker1/Development/local/wxt-without-hoisting/entrypoints/background.ts".

The error says it's coming from a virtual input. These inputs are shipped with WXT, but are not bundled until build time. This means they're effectively apart of your project, not WXT. That means they can't import any package not listed in your project's package.json, otherwise you get this error.

Solution: Export any utilities from WXT and import them from WXT in these virtual entrypoints instead of from the 3rd party package directly.

This may also work for webextension-polyfill, and we might not have to add it to the package.json of the templates. That would be ideal, because eventually, I want to remove the polyfill. If it's internal to WXT, I could remove it at anytime without making any breaking changes.

@aklinker1
Copy link
Collaborator

@rsweeney21 Can you try out v0.12.2-alpha2? I've got it working locally for some test projects.

@AndrewWalsh
Copy link
Contributor

@aklinker1 I encountered this issue today using the package manager pnpm and monorepo tool turbo. I installed 0.12.2-alpha2 and it worked for me.

@aklinker1
Copy link
Collaborator

Ok, I'll merge and release this today then!

@aklinker1
Copy link
Collaborator

Released in v0.12.2

@tesths
Copy link

tesths commented Jul 16, 2024

Today when I use wxt zip
The error is

 ERROR  [vite]: Rollup failed to resolve import "/Users/tesths/Desktop/prompts/entrypoints/background.ts-entrypoint" from "virtual:wxt-background?/Users/tesths/Desktop/prompts/entrypoints/background.ts".     23:00:59
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
build.rollupOptions.external

And my folder structure is

prompts/
     assets/
     entrypoints/
       content/
           index.tsx
       options/
           index.html
           main.tsx
       popup/
           index.html
           main.tsx
   background.ts

Is there any advice for solve it?

@aklinker1
Copy link
Collaborator

@tesths Can you open a new issue? Your problem is unrelated to this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants