fix(gatsby): fix webpack compilation when pnpm is used (#38757) (#38804)

* 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> (cherry picked from commit d2ffc2a) Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
gatsbyjs · Jan 10, 2024 · fc0eea1 · fc0eea1
1 parent f6ed443
commit fc0eea1
Show file tree

Hide file tree

Showing 9 changed files with 188 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,19 +378,23 @@ const createWebpackConfig = async ({
             },
           },
           use: {
-            loader: `babel-loader`,
+            loader: require.resolve(`babel-loader`),
             options: {
-              presets: [`@babel/typescript`],
+              presets: [
+                gatsbyPluginTSRequire.resolve(`@babel/preset-typescript`),
+              ],
             },
           },
         },
         {
           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
@@ -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 '<site-directory>/.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
     }