ai / size-limit

Calculate the real cost to run your JS app or lib to keep good performance. Show error in pull request if the cost exceeds the limit.
MIT License
6.48k stars 1.82k forks source link

CLI does not work with workspaces and yarn pnp or pnpm without hoisting #366

Open anomiex opened 3 weeks ago

anomiex commented 3 weeks ago

The way plugins and presets are loaded works with npm's hoisting, and even works in a non-monorepo (or where size-limit and its plugins are installed in the monorepo root), but does not work when size-limit is installed in a workspace with yarn pnp or pnpm with hoisting disabled.

Reproduction

With yarn:

  1. Create a temporary directory and cd into it.

  2. Create the following files:

    package.json ```json { "private": true, "workspaces": [ "a" ] } ```
    a/index.js Contents don't matter, even an empty file is ok.
    a/package.json ```json { "name": "a", "devDependencies": { "@size-limit/file": "^11.1.4", "size-limit": "^11.1.4" }, "size-limit": [ { "path": "index.js" } ], "scripts": { "size": "size-limit" } } ```
  3. Run yarn set version stable && yarn install

  4. Run yarn workspace a size

With pnpm:

  1. Create a temporary directory and cd into it.

  2. Create the following files:

    package.json ```json { } ```
    .npmrc ``` hoist-pattern=[] ```
    pnpm-workspace.yaml ```yaml packages: - 'a' ```
    a/index.js Contents don't matter, even an empty file is ok.
    a/package.json ```json { "name": "a", "devDependencies": { "@size-limit/file": "^11.1.4", "size-limit": "^11.1.4" }, "size-limit": [ { "path": "index.js" } ], "scripts": { "size": "size-limit" } } ```
  3. Run pnpm install.

  4. Run `pnpm

Expected behavior

size-limit runs successfully.

Actual behavior

With yarn:

 ERROR  Error: size-limit tried to access @size-limit/file, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: @size-limit/file (via "@size-limit/file/package.json")
Required by: size-limit@npm:11.1.4 (via /home/user/.yarn/berry/cache/size-limit-npm-11.1.4-dc2e299ec0-10c0.zip/node_modules/size-limit/load-plugins.js)

    at makeError (/tmp/test/.pnp.cjs:6744:34)
    at resolveToUnqualified (/tmp/test/.pnp.cjs:8392:21)
    at Object.resolveToUnqualified (/tmp/test/.pnp.cjs:8572:26)
    at resolve$1 (file:///tmp/test/.pnp.loader.mjs:2033:31)
    at nextResolve (node:internal/modules/esm/hooks:866:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:304:30)
    at MessagePort.handleMessage (node:internal/modules/esm/worker:196:24)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:825:20)
    at MessagePort.<anonymous> (node:internal/per_context/messageport:23:28)

With pnpm:

$ pnpm run --dir a size

> a@ size /tmp/test/a
> size-limit

 ERROR  Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@size-limit/file' imported from /tmp/test/node_modules/.pnpm/size-limit@11.1.4/node_modules/size-limit/load-plugins.js
    at packageResolve (node:internal/modules/esm/resolve:854:9)
    at moduleResolve (node:internal/modules/esm/resolve:927:18)
    at defaultResolve (node:internal/modules/esm/resolve:1157:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:383:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:352:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:227:38)
    at ModuleLoader.import (node:internal/modules/esm/loader:315:34)
    at importModuleDynamically (node:internal/modules/esm/translators:161:31)
    at importModuleDynamicallyCallback (node:internal/modules/esm/utils:222:14)
    at file:///tmp/test/node_modules/.pnpm/size-limit@11.1.4/node_modules/size-limit/load-plugins.js:26:27
 ELIFECYCLE  Command failed with exit code 1.
ai commented 3 weeks ago

Please, provide a solution suggestion

anomiex commented 3 weeks ago

Looks like it was broken by #345, using require.resolve with paths was avoiding this problem.

One possibility would be to partially undo that, but instead of require.resolve(i, { paths: [process.cwd()] }) try doing like import.meta.resolve( i, pathToFileURL( process.cwd() + '/package.json' ).toString() ) instead. That seems to work with my test cases above anyway.

Since the parent parameter to import.meta.resolve is still experimental, you'll probably want to use something like https://www.npmjs.com/package/import-meta-resolve to polyfill it.

ai commented 3 weeks ago

cc @JounQin

Since the parent parameter to import.meta.resolve is still experimental

If it is just a warning, it is OK. Does it work in ^18.0.0 || >=20.0.0?