Skip to content

Commit

Permalink
fix(gatsby): fix webpack compilation when pnpm is used (#38757)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
pieh and kathmbeck authored Jan 10, 2024
1 parent ce6b3bf commit d2ffc2a
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 24 deletions.
44 changes: 44 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 8 additions & 1 deletion packages/gatsby-dev-cli/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -154,4 +160,5 @@ watch(gatsbyLocation, argv.packages, {
monoRepoPackages,
packageNameToPath,
externalRegistry: argv.externalRegistry,
packageManager: argv.packageManager,
})
2 changes: 2 additions & 0 deletions packages/gatsby-dev-cli/src/local-npm-registry/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ exports.publishPackagesLocallyAndInstall = async ({
yarnWorkspaceRoot,
externalRegistry,
root,
packageManager,
}) => {
await startServer()

Expand All @@ -75,5 +76,6 @@ exports.publishPackagesLocallyAndInstall = async ({
yarnWorkspaceRoot,
newlyPublishedPackageVersions,
externalRegistry,
packageManager,
})
}
37 changes: 25 additions & 12 deletions packages/gatsby-dev-cli/src/local-npm-registry/install-packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const installPackages = async ({
yarnWorkspaceRoot,
newlyPublishedPackageVersions,
externalRegistry,
packageManager,
}) => {
console.log(
`Installing packages from local registry:\n${packagesToInstall
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down
3 changes: 3 additions & 0 deletions packages/gatsby-dev-cli/src/watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ async function watch(
localPackages,
packageNameToPath,
externalRegistry,
packageManager,
}
) {
setDefaultSpawnStdio(quiet ? `ignore` : `inherit`)
Expand Down Expand Up @@ -158,6 +159,7 @@ async function watch(
yarnWorkspaceRoot,
externalRegistry,
root,
packageManager,
})
} else {
// run `yarn`
Expand Down Expand Up @@ -344,6 +346,7 @@ async function watch(
ignorePackageJSONChanges,
externalRegistry,
root,
packageManager,
})
packagesToPublish.clear()
isPublishing = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
17 changes: 13 additions & 4 deletions packages/gatsby/src/internal-plugins/functions/gatsby-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -305,6 +306,10 @@ const createWebpackConfig = async ({
? `functions-production`
: `functions-development`

const gatsbyPluginTSRequire = mod.createRequire(
require.resolve(`gatsby-plugin-typescript`)
)

return {
entry: entries,
output: {
Expand Down Expand Up @@ -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`),
],
},
},
},
Expand Down
86 changes: 84 additions & 2 deletions packages/gatsby/src/utils/webpack/plugins/cache-folder-resolver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Resolver from "enhanced-resolve/lib/Resolver"
import mod from "module"

interface IRequest {
request?: string
Expand All @@ -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
}

Expand Down

0 comments on commit d2ffc2a

Please sign in to comment.