Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Jan 10, 2024
Merged

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

merged 9 commits into from
Jan 10, 2024

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Dec 15, 2023

Description

Our "cache folder resolver" (webpack plugin that resolves gatsby dependencies for files copied from gatsby package to .cache directory) is currently only being enabled for Yarn PnP.

process.versions.pnp check is only ever used for yarn, pnpm doesn't use so this PR adds more conditions that allow enablement of the plugin:

  • checking NODE_PATH env var - this is used when running things through pnpm CLI
  • finally we do just require.resolve checks to figure out wether required deps can be resolved from .cache directory - this path would be used when deps where installed with pnpm (tho it's possible other package managers can create node_modules structures that would hit same case too) and then regular gatsby CLI (and not via pnpm CLI) would be used to run gatsby - note that this path would NOT enable the plugin when using pnpm cli - because NODE_PATH is set the require.resolve tests would report that required deps are resolvable from .cache, but webpack resolver doesn't honor NODE_PATH so compilation would fail. (possible alternative would be to adjust webpack config to honor NODE_PATH but this is discouraged)

This fixes errors like:

ModuleNotFoundError: Module not found: Error: Can't resolve 'prop-types' in '<site-directory>/.cache'

when using pnpm (and possible others)

-- update

API functions compilation is also currently broken when using pnpm or yarn pnp so this is being fixed in this PR as well

Can be tested with gatsby@5.13.0-alpha-fix-pnpm.21 canary

Tests

Added test for pnpm similar as one we have for yarn PnP - tested in separate draft PRs:

Related Issues

https://linear.app/netlify/issue/FRA-179/fix-using-gatsby-with-pnpm

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 15, 2023
@pieh pieh added topic: webpack/babel Webpack or babel and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 15, 2023
cacheDirReq.resolve(cacheDirDep)
} catch {
// something is not resolable from the cache folder, so we should not enable this plugin
isEverythingResolvableFromCacheDir = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is false, could break out of the for loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we would need 2 separate loops for checking if tested dependency is resolvable from .cache and from `gatsby package - which I think probably makes more sense than current single loop, so I will make the change to run those checks in separate loops and just break the loops on first unresolvable module

pieh and others added 3 commits January 9, 2024 09:58
…r.ts

Co-authored-by: Katherine Beck <49894658+kathmbeck@users.noreply.github.com>
…d gatsby package, break on first unresolvable module
@pieh pieh merged commit d2ffc2a into master Jan 10, 2024
34 checks passed
@pieh pieh deleted the fix/pnpm branch January 10, 2024 07:22
@pieh pieh added this to To cherry-pick in V5 Release hotfixes via automation Jan 10, 2024
pieh added a commit that referenced this pull request Jan 10, 2024
* 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)
pieh added a commit that referenced this pull request Jan 10, 2024
* 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>
@pieh pieh moved this from To cherry-pick to Backported in V5 Release hotfixes Jan 10, 2024
@pieh pieh moved this from Backported to Published in V5 Release hotfixes Jan 23, 2024
This was referenced May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: webpack/babel Webpack or babel
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants