From d2ffc2a348fd62f9b0f887f860d415c813f9cbac Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 10 Jan 2024 08:22:37 +0100 Subject: [PATCH] fix(gatsby): fix webpack compilation when pnpm is used (#38757) * fix: use different method of figuring out wether to enable .cache resolver plugin * use multiple conditions when deciding wether to enable .cache folder resolver plugin * feat(gatsby-dev-cli): allow using pnpm for installing deps * test: add pnpm test * test: test api functions for pnp and pnpm * fix: api functions * Update packages/gatsby/src/utils/webpack/plugins/cache-folder-resolver.ts Co-authored-by: Katherine Beck <49894658+kathmbeck@users.noreply.github.com> * chore: separate test loops for trying to resolve from cache folder and gatsby package, break on first unresolvable module --------- Co-authored-by: Katherine Beck <49894658+kathmbeck@users.noreply.github.com> --- .circleci/config.yml | 44 ++++++++++ packages/gatsby-dev-cli/src/index.js | 9 +- .../src/local-npm-registry/index.js | 2 + .../local-npm-registry/install-packages.js | 37 +++++--- .../local-npm-registry/verdaccio-config.js | 2 +- packages/gatsby-dev-cli/src/watch.js | 3 + .../functions/api-function-webpack-loader.ts | 12 ++- .../internal-plugins/functions/gatsby-node.ts | 17 +++- .../webpack/plugins/cache-folder-resolver.ts | 86 ++++++++++++++++++- 9 files changed, 188 insertions(+), 24 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b9d39a3f86996..1d517795f434c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -341,6 +341,9 @@ jobs: - run: command: cp -r ./starters/default /tmp/e2e-tests/gatsby-pnp working_directory: ~/project + - run: # default doesn't have API functions so let's get some of those + command: cp -r ./e2e-tests/adapters/src/api /tmp/e2e-tests/gatsby-pnp/src/api + working_directory: ~/project - run: command: touch yarn.lock working_directory: /tmp/e2e-tests/gatsby-pnp @@ -379,6 +382,45 @@ jobs: command: 'DEBUG=start-server-and-test yarn start-server-and-test "yarn develop 2>&1 | tee log.txt" :8000 "! cat log.txt | grep -E ''ERROR #|Require stack:''"' working_directory: /tmp/e2e-tests/gatsby-pnp + e2e_tests_pnpm: + executor: + name: node + image: "18.12.0" + steps: + - checkout + - run: ./scripts/assert-changed-files.sh "packages/*|.circleci/*" + - <<: *attach_to_bootstrap + - run: + command: mkdir -p /tmp/e2e-tests/ + working_directory: ~/project + - run: + command: cp -r ./starters/default /tmp/e2e-tests/gatsby-pnpm + working_directory: ~/project + - run: # default doesn't have API functions so let's get some of those + command: cp -r ./e2e-tests/adapters/src/api /tmp/e2e-tests/gatsby-pnpm/src/api + working_directory: ~/project + - run: + command: rm package-lock.json + working_directory: /tmp/e2e-tests/gatsby-pnpm + - run: # Install pnpm + command: npm install -g pnpm + working_directory: /tmp/e2e-tests/gatsby-pnpm + - run: # Install start-server-and-test + command: npm install -g start-server-and-test@^1.11.0 + working_directory: /tmp/e2e-tests/gatsby-pnpm + - run: # Set project dir + command: node ~/project/packages/gatsby-dev-cli/dist/index.js --set-path-to-repo ~/project + working_directory: /tmp/e2e-tests/gatsby-pnpm + - run: # Copy over packages + command: node ~/project/packages/gatsby-dev-cli/dist/index.js --force-install --scan-once --package-manager pnpm + working_directory: /tmp/e2e-tests/gatsby-pnpm + - run: + command: pnpm build + working_directory: /tmp/e2e-tests/gatsby-pnpm + - run: + command: 'DEBUG=start-server-and-test pnpm start-server-and-test "pnpm develop 2>&1 | tee log.txt" :8000 "! cat log.txt | grep -E ''ERROR #|Require stack:''"' + working_directory: /tmp/e2e-tests/gatsby-pnpm + e2e_tests_development_runtime_with_react_18: <<: *e2e-executor steps: @@ -606,6 +648,8 @@ workflows: - bootstrap - e2e_tests_pnp: <<: *e2e-test-workflow + - e2e_tests_pnpm: + <<: *e2e-test-workflow - e2e_tests_path-prefix: <<: *e2e-test-workflow - e2e_tests_visual-regression: diff --git a/packages/gatsby-dev-cli/src/index.js b/packages/gatsby-dev-cli/src/index.js index ebbc20f3474c7..e72b03f5258c3 100644 --- a/packages/gatsby-dev-cli/src/index.js +++ b/packages/gatsby-dev-cli/src/index.js @@ -44,7 +44,13 @@ You typically only need to configure this once.` .alias(`h`, `help`) .nargs(`v`, 0) .alias(`v`, `version`) - .describe(`v`, `Print the currently installed version of Gatsby Dev CLI`).argv + .describe(`v`, `Print the currently installed version of Gatsby Dev CLI`) + .choices(`package-manager`, [`yarn`, `pnpm`]) + .default(`package-manager`, `yarn`) + .describe( + `package-manager`, + `Package manager to use for installing dependencies.` + ).argv if (argv.version) { console.log(getVersionInfo()) @@ -154,4 +160,5 @@ watch(gatsbyLocation, argv.packages, { monoRepoPackages, packageNameToPath, externalRegistry: argv.externalRegistry, + packageManager: argv.packageManager, }) diff --git a/packages/gatsby-dev-cli/src/local-npm-registry/index.js b/packages/gatsby-dev-cli/src/local-npm-registry/index.js index ec44979c5ec80..210312ebabb05 100644 --- a/packages/gatsby-dev-cli/src/local-npm-registry/index.js +++ b/packages/gatsby-dev-cli/src/local-npm-registry/index.js @@ -50,6 +50,7 @@ exports.publishPackagesLocallyAndInstall = async ({ yarnWorkspaceRoot, externalRegistry, root, + packageManager, }) => { await startServer() @@ -75,5 +76,6 @@ exports.publishPackagesLocallyAndInstall = async ({ yarnWorkspaceRoot, newlyPublishedPackageVersions, externalRegistry, + packageManager, }) } diff --git a/packages/gatsby-dev-cli/src/local-npm-registry/install-packages.js b/packages/gatsby-dev-cli/src/local-npm-registry/install-packages.js index b968a0b8a8eb1..3ca0bd84f165e 100644 --- a/packages/gatsby-dev-cli/src/local-npm-registry/install-packages.js +++ b/packages/gatsby-dev-cli/src/local-npm-registry/install-packages.js @@ -9,6 +9,7 @@ const installPackages = async ({ yarnWorkspaceRoot, newlyPublishedPackageVersions, externalRegistry, + packageManager, }) => { console.log( `Installing packages from local registry:\n${packagesToInstall @@ -127,20 +128,32 @@ const installPackages = async ({ installCmd = [`yarn`, yarnCommands] } else { - const yarnCommands = [ - `add`, - ...packagesToInstall.map(packageName => { - const packageVersion = newlyPublishedPackageVersions[packageName] - return `${packageName}@${packageVersion}` - }), - `--exact`, - ] + const packageAndVersionsToInstall = packagesToInstall.map(packageName => { + const packageVersion = newlyPublishedPackageVersions[packageName] + return `${packageName}@${packageVersion}` + }) - if (!externalRegistry) { - yarnCommands.push(`--registry=${registryUrl}`) - } + if (packageManager === `pnpm`) { + const pnpmCommands = [ + `add`, + ...packageAndVersionsToInstall, + `--save-exact`, + ] - installCmd = [`yarn`, yarnCommands] + if (!externalRegistry) { + pnpmCommands.push(`--registry=${registryUrl}`) + } + + installCmd = [`pnpm`, pnpmCommands] + } else { + const yarnCommands = [`add`, ...packageAndVersionsToInstall, `--exact`] + + if (!externalRegistry) { + yarnCommands.push(`--registry=${registryUrl}`) + } + + installCmd = [`yarn`, yarnCommands] + } } try { diff --git a/packages/gatsby-dev-cli/src/local-npm-registry/verdaccio-config.js b/packages/gatsby-dev-cli/src/local-npm-registry/verdaccio-config.js index 3cd492a0cfe68..bbbc363460add 100644 --- a/packages/gatsby-dev-cli/src/local-npm-registry/verdaccio-config.js +++ b/packages/gatsby-dev-cli/src/local-npm-registry/verdaccio-config.js @@ -10,7 +10,7 @@ const verdaccioConfig = { title: `gatsby-dev`, }, self_path: `./`, - logs: [{ type: `stdout`, format: `pretty-timestamped`, level: `warn` }], + logs: { type: `stdout`, format: `pretty-timestamped`, level: `warn` }, packages: { "**": { access: `$all`, diff --git a/packages/gatsby-dev-cli/src/watch.js b/packages/gatsby-dev-cli/src/watch.js index 94134dc842641..3b3d1b04d7659 100644 --- a/packages/gatsby-dev-cli/src/watch.js +++ b/packages/gatsby-dev-cli/src/watch.js @@ -38,6 +38,7 @@ async function watch( localPackages, packageNameToPath, externalRegistry, + packageManager, } ) { setDefaultSpawnStdio(quiet ? `ignore` : `inherit`) @@ -158,6 +159,7 @@ async function watch( yarnWorkspaceRoot, externalRegistry, root, + packageManager, }) } else { // run `yarn` @@ -344,6 +346,7 @@ async function watch( ignorePackageJSONChanges, externalRegistry, root, + packageManager, }) packagesToPublish.clear() isPublishing = false diff --git a/packages/gatsby/src/internal-plugins/functions/api-function-webpack-loader.ts b/packages/gatsby/src/internal-plugins/functions/api-function-webpack-loader.ts index fb7bca935e46c..2642f691431c8 100644 --- a/packages/gatsby/src/internal-plugins/functions/api-function-webpack-loader.ts +++ b/packages/gatsby/src/internal-plugins/functions/api-function-webpack-loader.ts @@ -13,10 +13,14 @@ const APIFunctionLoader: LoaderDefinition = async function () { const functionModule = require('${modulePath}'); const functionToExecute = preferDefault(functionModule); const matchPath = '${matchPath}'; - const { match: reachMatch } = require('@gatsbyjs/reach-router'); - const { urlencoded, text, json, raw } = require('body-parser') - const multer = require('multer') - const { createConfig } = require('gatsby/dist/internal-plugins/functions/config') + const { match: reachMatch } = require('${require.resolve( + `@gatsbyjs/reach-router` + )}'); + const { urlencoded, text, json, raw } = require('${require.resolve( + `body-parser` + )}') + const multer = require('${require.resolve(`multer`)}') + const { createConfig } = require('${require.resolve(`./config`)}') function functionWrapper(req, res) { if (matchPath) { diff --git a/packages/gatsby/src/internal-plugins/functions/gatsby-node.ts b/packages/gatsby/src/internal-plugins/functions/gatsby-node.ts index 5f4b5b8141d0e..910303d030de5 100644 --- a/packages/gatsby/src/internal-plugins/functions/gatsby-node.ts +++ b/packages/gatsby/src/internal-plugins/functions/gatsby-node.ts @@ -12,6 +12,7 @@ import { reportWebpackWarnings } from "../../utils/webpack-error-utils" import { internalActions } from "../../redux/actions" import { IGatsbyFunction } from "../../redux/types" import { functionMiddlewares } from "./middleware" +import mod from "module" const isProductionEnv = process.env.gatsby_executing_command !== `develop` @@ -305,6 +306,10 @@ const createWebpackConfig = async ({ ? `functions-production` : `functions-development` + const gatsbyPluginTSRequire = mod.createRequire( + require.resolve(`gatsby-plugin-typescript`) + ) + return { entry: entries, output: { @@ -373,9 +378,11 @@ const createWebpackConfig = async ({ }, }, use: { - loader: `babel-loader`, + loader: require.resolve(`babel-loader`), options: { - presets: [`@babel/typescript`], + presets: [ + gatsbyPluginTSRequire.resolve(`@babel/preset-typescript`), + ], }, }, }, @@ -383,9 +390,11 @@ const createWebpackConfig = async ({ test: [/.js$/, /.ts$/], exclude: /node_modules/, use: { - loader: `babel-loader`, + loader: require.resolve(`babel-loader`), options: { - presets: [`@babel/typescript`], + presets: [ + gatsbyPluginTSRequire.resolve(`@babel/preset-typescript`), + ], }, }, }, diff --git a/packages/gatsby/src/utils/webpack/plugins/cache-folder-resolver.ts b/packages/gatsby/src/utils/webpack/plugins/cache-folder-resolver.ts index d71cb513356a9..7fde7fe08da3f 100644 --- a/packages/gatsby/src/utils/webpack/plugins/cache-folder-resolver.ts +++ b/packages/gatsby/src/utils/webpack/plugins/cache-folder-resolver.ts @@ -1,4 +1,5 @@ import Resolver from "enhanced-resolve/lib/Resolver" +import mod from "module" interface IRequest { request?: string @@ -8,17 +9,98 @@ interface IRequest { type ProcessWithPNP = NodeJS.ProcessVersions & { pnp?: string } /** - * To support PNP we have to make sure dependencies resolved from the .cache folder should be resolved from the gatsby package directory + * To support Yarn PNP and pnpm we have to make sure dependencies resolved from + * the .cache folder should be resolved from the gatsby package directory + * If you see error like + * + * ModuleNotFoundError: Module not found: Error: Can't resolve 'prop-types' + * in '/.cache' + * + * it probably means this plugin is not enabled when it should be and there + * might be need to adjust conditions for setting `this.isEnabled` in the + * constructor. + * + * It's not enabled always because of legacy behavior and to limit potential + * regressions. Might be good idea to enable it always in the future + * OR remove the need for the plugin completely by not copying `cache-dir` + * contents to `.cache` folder and instead adjust setup to use those browser/node + * html renderer runtime files directly from gatsby package */ export class CacheFolderResolver { private requestsFolder: string + private isEnabled = false constructor(requestsFolder: string) { this.requestsFolder = requestsFolder + + if ((process.versions as ProcessWithPNP).pnp) { + // Yarn PnP + this.isEnabled = true + } else if (/node_modules[/\\]\.pnpm/.test(process.env.NODE_PATH ?? ``)) { + // pnpm when executing through `pnpm` CLI + this.isEnabled = true + } else { + // pnpm when executing through regular `gatsby` CLI / `./node_modules/.bin/gatsby` + // would not set NODE_PATH, but node_modules structure would not allow to resolve + // gatsby deps from the cache folder (unless user would install same deps too) + // so we are checking if we can resolve deps from the cache folder + // this check is not limited to pnpm and other package managers could hit this path too + + // Hardcoded list of gatsby deps used in gatsby browser and ssr runtimes + // instead of checking if we use Yarn PnP (via `process.versions.pnp`), + // we check if we can resolve the external deps from the cache-dir folder + // to know if we need to enable this plugin so we also cover pnpm + // It might be good idea to always enable it overall, but to limit potential + // regressions we only enable it if we are sure we need it. + const modulesToCheck = [ + `prop-types`, + `lodash/isEqual`, + `mitt`, + `shallow-compare`, + `@gatsbyjs/reach-router`, + `gatsby-react-router-scroll`, + `react-server-dom-webpack`, + `gatsby-link`, + ] + + // test if we can resolve deps from the cache folder + let isEverythingResolvableFromCacheDir = true + const cacheDirReq = mod.createRequire(requestsFolder) + for (const cacheDirDep of modulesToCheck) { + try { + cacheDirReq.resolve(cacheDirDep) + } catch { + // something is not resolvable from the cache folder, so we should not enable this plugin + isEverythingResolvableFromCacheDir = false + break + } + } + + // test if we can resolve deps from the gatsby package + let isEverythingResolvableFromGatsbyPackage = true + for (const cacheDirDep of modulesToCheck) { + try { + require.resolve(cacheDirDep) + } catch { + // something is not resolvable from the gatsby package + isEverythingResolvableFromGatsbyPackage = false + break + } + } + + // we only enable this plugin if we are unable to resolve cache-dir deps from .cache folder + // and we can resolve them from gatsby package + if ( + !isEverythingResolvableFromCacheDir && + isEverythingResolvableFromGatsbyPackage + ) { + this.isEnabled = true + } + } } apply(resolver: Resolver): void { - if (!(process.versions as ProcessWithPNP).pnp) { + if (!this.isEnabled) { return }