roots / bud

High-performance build system that supports SWC, esbuild, and Babel
https://bud.js.org/
MIT License
325 stars 38 forks source link

[request] does not work with pnpm #1945

Closed debuggerpk closed 1 year ago

debuggerpk commented 1 year ago

Agreement

Describe the issue

We have a monorepo where we are using pnpm. Due to project needs, we need to wordpress to the mix. We started with trellis, and then sage .. onto bud. by default pnpm did not work. I had to manually update dev dependencies to following to make it work.

{
    "@babel/plugin-proposal-class-properties": "^7.18.6",
    "@babel/plugin-proposal-object-rest-spread": "^7.20.7",
    "@babel/plugin-syntax-dynamic-import": "^7.8.3",
    "@babel/plugin-transform-runtime": "^7.19.6",
    "@babel/preset-env": "^7.20.2",
    "@babel/preset-react": "^7.18.6",
    "@pmmmwh/react-refresh-webpack-plugin": "^0.5.7",
    "@roots/bud": "6.6.6",
    "@roots/bud-babel": "6.6.6",
    "@roots/bud-client": "6.6.6",
    "@roots/bud-entrypoints": "6.6.6",
    "@roots/bud-extensions": "6.6.6",
    "@roots/bud-postcss": "6.6.6",
    "@roots/bud-preset-recommend": "6.6.6",
    "@roots/bud-preset-wordpress": "6.6.6",
    "@roots/bud-react": "6.6.6",
    "@roots/bud-server": "6.6.6",
    "@roots/bud-support": "6.6.6",
    "@roots/bud-tailwindcss": "6.6.6",
    "@roots/bud-wordpress-dependencies": "6.6.6",
    "@roots/bud-wordpress-externals": "6.6.6",
    "@roots/bud-wordpress-manifests": "6.6.6",
    "@roots/merged-manifest-webpack-plugin": "6.6.6",
    "@roots/sage": "6.6.6",
    "@roots/wordpress-dependencies-webpack-plugin": "6.6.6",
    "@roots/wordpress-externals-webpack-plugin": "6.6.6",
    "babel-loader": "^9.1.0",
    "clean-webpack-plugin": "^4.0.0",
    "copy-webpack-plugin": "11.0.0",
    "css-loader": "^6.7.3",
    "express": "4.18.1",
    "html-webpack-plugin": "5.5.0",
    "postcss": "^8.4.14",
    "postcss-import": "^14.1.0",
    "postcss-js": "^4.0.0",
    "postcss-load-config": "^3.1.4",
    "postcss-loader": "7.0.1",
    "postcss-nested": "5.0.6",
    "postcss-preset-env": "^7.8.2",
    "postcss-selector-parser": "^6.0.10",
    "postcss-value-parser": "^4.2.0",
    "style-loader": "^3.3.1",
    "tailwindcss": "^3.1.8",
    "webpack-manifest-plugin": "^5.0.0"
  }

Expected Behavior

It should work pnpm regardless.

Actual Behavior

gave me multiple errors and i have to add dependencies step by step.

Steps To Reproduce

composer create-project roots/sage sage
cd sage
pnpm install & pnpm dev

version

6.6.2

What package manager are you using?

yarn classic

version

pnpm

Logs

No response

Configuration

// @ts-check

/**
 * Build configuration
 *
 * @see {@link https://bud.js.org/guides/configure}
 * @param {import('@roots/bud').Bud} bud
 */
export default async bud => {
  bud
    /**
     * Application entrypoints
     */
    .entry({
      app: ['@scripts/app', '@styles/app'],
      editor: ['@scripts/editor', '@styles/editor'],
    })

    /**
     * Directory contents to be included in the compilation
     */
    .assets(['images'])

    /**
     * Matched files trigger a page reload when modified
     */
    .watch(['resources/views/**/*', 'app/**/*'])

    /**
     * Proxy origin (`WP_HOME`)
     */
    .proxy('http://localhost:8080')

    /**
     * Development origin
     */
    .serve('http://0.0.0.0:3000')

    /**
     * URI of the `public` directory
     */
    .setPublicPath('/app/themes/default/public/')

    /**
     * Generate WordPress `theme.json`
     *
     * @note This overwrites `theme.json` on every build.
     */
    .wpjson.settings({
      color: {
        custom: false,
        customGradient: false,
        defaultPalette: false,
        defaultGradients: false,
      },
      custom: {
        spacing: {},
        typography: {
          'font-size': {},
          'line-height': {},
        },
      },
      spacing: {
        padding: true,
        units: ['px', '%', 'em', 'rem', 'vw', 'vh'],
      },
      typography: {
        customFontSize: false,
      },
    })
    .useTailwindColors()
    .useTailwindFontFamily()
    .useTailwindFontSize()
    .enable();
};

Relevant .budfiles

No response

kellymears commented 1 year ago

It seems to work with the following .npmrc settings:

auto-install-peers=true
shamefully-hoist=true

(I didn't feel any shame)

I'm admittedly pretty ignorant of pnpm but one of the main selling points of pnpm seems to be that it makes dependencies explicit rather than implicit (they shame you!). So, isn't this kind of expected? I would imagine that using bud.js with pnpm is going to involve a certain amount of this sort of manual peer dependency maintenance.

I'd want to work on making bud.js yarn pnp compatible before working pnpm support, but I'm open to PRs if this is important enough for someone else to take on.

kellymears commented 1 year ago

The more I think about this the more I think everything is working exactly as intended for both tools (bud.js and pnpm). As an example, here's how bud.js declares its peer dependencies in @roots/bud-babel:

{
  "peerDependencies": {
    "@babel/core": "*",
    "@babel/plugin-proposal-class-properties": "*",
    "@babel/plugin-proposal-object-rest-spread": "*",
    "@babel/plugin-syntax-dynamic-import": "*",
    "@babel/plugin-transform-runtime": "*",
    "@babel/preset-env": "*",
    "babel-loader": "*"
  },
  "peerDependenciesMeta": {
    "@babel/core": {
      "optional": true
    },
    "@babel/plugin-proposal-class-properties": {
      "optional": true
    },
    "@babel/plugin-proposal-object-rest-spread": {
      "optional": true
    },
    "@babel/plugin-syntax-dynamic-import": {
      "optional": true
    },
    "@babel/plugin-transform-runtime": {
      "optional": true
    },
    "@babel/preset-env": {
      "optional": true
    },
    "babel-loader": {
      "optional": true
    }
  }
}

This gives users the flexibility to override those peer dependencies (if they choose). It works great for yarn and npm. I think it also works for pnpm but maybe not in the way that you'd like:

https://pnpm.io/motivation#creating-a-non-flat-node_modules-directory

In the case of pnpm working well with bud.js dependency resolution the options seem to be either:

But, again, I'd be open to a PR if someone feels I've missed something.

debuggerpk commented 1 year ago

Thanks @kellymears for the response.

kellymears commented 1 year ago

@debuggerpk when i install 6.6.10 with pnpm:

Progress: resolved 982, reused 884, downloaded 98, added 984, done
node_modules/.pnpm/core-js-pure@3.27.1/node_modules/core-js-pure: Running postinstall script, done in 48ms
node_modules/.pnpm/@roots+bud@6.6.10/node_modules/@roots/bud: Running postinstall script, done in 10s

devDependencies:
+ @roots/bud 6.6.10
+ @roots/bud-tailwindcss 6.6.10
+ @roots/sage 6.6.10

 WARN  Issues with peer dependencies found
.
├─┬ @roots/bud 6.6.10
│ └─┬ @roots/bud-entrypoints 6.6.10
│   └─┬ @roots/entrypoints-webpack-plugin 6.6.10
│     └── ✕ missing peer webpack@>=5
├─┬ @roots/bud-tailwindcss 6.6.10
│ └─┬ @roots/bud-postcss 6.6.10
│   └─┬ postcss-loader 7.0.2
│     └── ✕ missing peer webpack@^5.0.0
└─┬ @roots/sage 6.6.10
  └─┬ @roots/bud-preset-wordpress 6.6.10
    ├─┬ @roots/bud-preset-recommend 6.6.10
    │ └─┬ @roots/bud-babel 6.6.10
    │   └─┬ babel-loader 9.1.0
    │     └── ✕ missing peer webpack@>=5
    └─┬ @roots/bud-react 6.6.10
      ├── ✕ missing peer @babel/core@"*"
      ├─┬ @pmmmwh/react-refresh-webpack-plugin 0.5.10
      │ └── ✕ missing peer webpack@">=4.43.0 <6.0.0"
      └─┬ @babel/preset-react 7.18.6
        ├── ✕ missing peer @babel/core@^7.0.0-0
        ├─┬ @babel/plugin-transform-react-display-name 7.18.6
        │ └── ✕ missing peer @babel/core@^7.0.0-0
        ├─┬ @babel/plugin-transform-react-jsx 7.20.7
        │ ├── ✕ missing peer @babel/core@^7.0.0-0
        │ └─┬ @babel/plugin-syntax-jsx 7.18.6
        │   └── ✕ missing peer @babel/core@^7.0.0-0
        ├─┬ @babel/plugin-transform-react-jsx-development 7.18.6
        │ └── ✕ missing peer @babel/core@^7.0.0-0
        └─┬ @babel/plugin-transform-react-pure-annotations 7.18.6
          └── ✕ missing peer @babel/core@^7.0.0-0
Peer dependencies that should be installed:
  @babel/core@">=7.0.0 <8.0.0"  webpack@">=5.0.0 <6.0.0"

The integrity of 5684 files was checked. This might have caused installation to take longer.
Done in 20.6s

But, it also works despite the warning message. Has there been an improvement for you?

montchr commented 1 year ago

I've been running into this too, but with a blocking error even after installing those peer dependencies:

devDependencies:
+ @babel/core 7.21.0
+ @babel/plugin-proposal-class-properties 7.18.6
+ @babel/plugin-proposal-object-rest-spread 7.20.7
+ @babel/plugin-syntax-dynamic-import 7.8.3
+ @babel/plugin-transform-runtime 7.21.0
+ @babel/preset-env 7.20.2
+ babel-loader 9.1.2

Done in 5.2s
6.23s user 1.92s system 152% cpu 5.348s total

app/themes/logan-center-theme on  appdir-vn [  ]                                                                                                                    via  18 via  v8.0.27 took 5s
❯ pnpm build

> @kleinweb/logan-center-wordpress-theme@ build /Users/cdom/Developer/work/projects/logan-center/apps/wordpress/web/app/themes/logan-center-theme
> bud build

node:internal/errors:490
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'typanion' imported from /Users/cdom/Developer/work/projects/logan-center/node_modules/.pnpm/@roots+bud-support@6.9.1_webpack@5.75.0/node_modules/@roots/bud-support/lib/typanion/index.js
    at new NodeError (node:internal/errors:399:5)
    at packageResolve (node:internal/modules/esm/resolve:889:9)
    at moduleResolve (node:internal/modules/esm/resolve:938:20)
    at defaultResolve (node:internal/modules/esm/resolve:1153:11)
    at nextResolve (node:internal/modules/esm/loader:163:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:838:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:77:40)
    at link (node:internal/modules/esm/module_job:76:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Node.js v18.14.1
kellymears commented 1 year ago

install with the --shamefully-hoist flag and it works (6.11.0) https://pnpm.io/npmrc#shamefully-hoist

❯ pnpm install --shamefully-hoist      
 WARN  deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
Packages: +991
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Packages are hard linked from the content-addressable store to the virtual store.
  Content-addressable store is at: /Users/kellymears/.local/share/pnpm/store/v3
  Virtual store is at:             node_modules/.pnpm
Progress: resolved 990, reused 990, downloaded 0, added 991, done

devDependencies:
+ @roots/bud 6.11.0
+ @roots/bud-tailwindcss 6.11.0
+ @roots/sage 6.11.0

 WARN  Issues with peer dependencies found
.
├─┬ @roots/bud-tailwindcss 6.11.0
│ └─┬ @roots/bud-postcss 6.11.0
│   ├── ✕ missing peer webpack@5.75.0
│   └─┬ postcss-loader 7.0.2
│     └── ✕ missing peer webpack@^5.0.0
└─┬ @roots/sage 6.11.0
  └─┬ @roots/bud-preset-wordpress 6.11.0
    ├─┬ @roots/bud-preset-recommend 6.11.0
    │ └─┬ @roots/bud-babel 6.11.0
    │   ├── ✕ missing peer webpack@"*"
    │   └─┬ babel-loader 9.1.2
    │     └── ✕ missing peer webpack@>=5
    └─┬ @roots/bud-react 6.11.0
      ├── ✕ missing peer webpack@"*"
      ├── ✕ missing peer @babel/core@"*"
      ├─┬ @pmmmwh/react-refresh-webpack-plugin 0.5.10
      │ └── ✕ missing peer webpack@">=4.43.0 <6.0.0"
      └─┬ @babel/preset-react 7.18.6
        ├── ✕ missing peer @babel/core@^7.0.0-0
        ├─┬ @babel/plugin-transform-react-display-name 7.18.6
        │ └── ✕ missing peer @babel/core@^7.0.0-0
        ├─┬ @babel/plugin-transform-react-jsx 7.21.0
        │ ├── ✕ missing peer @babel/core@^7.0.0-0
        │ └─┬ @babel/plugin-syntax-jsx 7.18.6
        │   └── ✕ missing peer @babel/core@^7.0.0-0
        ├─┬ @babel/plugin-transform-react-jsx-development 7.18.6
        │ └── ✕ missing peer @babel/core@^7.0.0-0
        └─┬ @babel/plugin-transform-react-pure-annotations 7.18.6
          └── ✕ missing peer @babel/core@^7.0.0-0
Peer dependencies that should be installed:
  @babel/core@">=7.0.0 <8.0.0"  webpack@5.75.0

❯ pnpm bud build    

╭─ sage ./public [04ec2d508357ac7c48a2]
│
├─ entrypoints
│  ├─ app
│  │  ├─ js/runtime.06fab3.js     1.05 kB
│  │  ├─ js/459.300ec7.js       370 bytes
│  │  ├─ css/app.6a0078.css       5.24 kB
│  │  └─ js/app.093d38.js       395 bytes
│  └─ editor
│     ├─ js/runtime.06fab3.js     1.05 kB
│     ├─ js/459.300ec7.js       370 bytes
│     ├─ css/editor.31d6cf.css    0 bytes
│     └─ js/editor.850142.js    559 bytes
│
╰─ compiled 24 modules in 1s 83ms

Reiterating, pnpm's module resolution strategy seems somewhat at odds with our strategy, which is:

If you don't want to use the --shamefully-hoist flag, which near as I can tell just restores the normal hoisting strategy of npm and yarn, then you'll need to install typanion and whatever else comes up subsequently in error messages in order to guarantee they are hoisted and resolvable.

kellymears commented 1 year ago

Updated docs on pnpm compatibility: https://bud.js.org/guides/general-use/pnpm

kellymears commented 1 year ago

I have no problems installing roots/sage with pnpm in 6.12.3, which includes one final batch of pnpm specific changes.

You must install with @roots/* packages hoisted to public scope.

pnpm install --public-hoist-pattern="@roots/*"

If you have any issues you will need to install with all packages in public scope:

pnpm install --public-hoist-pattern="*"

In that case, you might want to make sure your application dependencies are explicitly defined (the problem may not lie with bud.js).

Either way, the .pnpmfile from the docs should no longer be necesssary. Here's my terminal output when running the install:

❯ pnpm install --public-hoist-pattern="*"                 

   ╭─────────────────────────────────────────────────────────────────╮
   │                                                                 │
   │                Update available! 8.1.1 → 8.5.0.                 │
   │   Changelog: https://github.com/pnpm/pnpm/releases/tag/v8.5.0   │
   │                Run "pnpm add -g pnpm" to update.                │
   │                                                                 │
   │     Follow @pnpmjs for updates: https://twitter.com/pnpmjs      │
   │                                                                 │
   ╰─────────────────────────────────────────────────────────────────╯

...

Packages: +1014
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Packages are copied from the content-addressable store to the virtual store.
  Content-addressable store is at: ...
  Virtual store is at:             ...
...

devDependencies:
+ @roots/bud 6.12.3
+ @roots/bud-tailwindcss 6.12.3
+ @roots/sage 6.12.3

The integrity of 2895 files was checked. This might have caused installation to take longer.

❯ pnpm bud build                             

╭─ sage ./public [fffb86bfd9cd7e1119d1]
│
├─ entrypoints
│  ├─ app
│  │  ├─ js/runtime.00900e.js     1.22 kB
│  │  ├─ js/691.b2b4ae.js       391 bytes
│  │  ├─ css/app.9e57b3.css       5.27 kB
│  │  └─ js/app.78899e.js       416 bytes
│  └─ editor
│     ├─ js/runtime.00900e.js     1.22 kB
│     ├─ js/284.1eebae.js       990 bytes
│     ├─ css/editor.31d6cf.css    0 bytes
│     └─ js/editor.a0539f.js      1.25 kB
│
╰─ compiled 32 modules (4 cached) in 1s 300ms

Feel free to make new issues if you have problems with any specific @roots/* packages not installing properly with pnpm. But, I'm marking this as closed.