Skip to content

Framework: Switch from hoisted to isolated dependencies #83847

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.

Sign up for GitHub

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

Merged
manzoorwanijk merged 6 commits into from
Oct 1, 2026
Merged

Framework: Switch from hoisted to isolated dependencies #83847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
55239ed
Framework: Switch to npm install-strategy=linked
manzoorwanijk Sep 14, 2026
0e5a012
Framework: Adapt tooling to the isolated install layout
manzoorwanijk Sep 30, 2026
d086108
Framework: Require npm v11.18
manzoorwanijk Sep 30, 2026
bbb6808
Build: Detect a node_modules installed with the other strategy
manzoorwanijk Oct 1, 2026
dc63a44
Storybook: Restore docgen coverage under the isolated layout
manzoorwanijk Oct 1, 2026
4564123
Framework: Drop the NODE_PATH fallback in create-block validation
manzoorwanijk Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .github/setup-node/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ runs:
echo "NPM_VERSION=$(npm -v)" >> "$GITHUB_OUTPUT"
shell: bash

# Windows tar cannot restore the isolated layout's links, so always install there.
- name: Cache node_modules
id: cache-node_modules
if: ${{ inputs.cache == 'true' }}
if: ${{ inputs.cache == 'true' && runner.os != 'Windows' }}
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
# The key covers .npmrc because install-strategy decides the layout:
# a hoisted tree restored into an isolated checkout resolves nothing.
# List workspace node_modules explicitly to support all kinds of layouts - hoisted and isolated.
# A '**/node_modules' glob follows the isolated layout's symlinks and stalls the cache step.
# The eval package is not a workspace: it installs its own large tree from its own lockfile.
Expand All @@ -49,7 +52,7 @@ runs:
!test/ai-development/node_modules
tools/*/node_modules
widgets/*/node_modules
key: node_modules-${{ runner.os }}-${{ runner.arch }}-${{ steps.node-version.outputs.NODE_VERSION }}-npm-${{ steps.node-version.outputs.NPM_VERSION }}-${{ hashFiles('package-lock.json', 'patches/**') }}
key: node_modules-${{ runner.os }}-${{ runner.arch }}-${{ steps.node-version.outputs.NODE_VERSION }}-npm-${{ steps.node-version.outputs.NPM_VERSION }}-${{ hashFiles('package-lock.json', '.npmrc') }}

- name: Install npm dependencies
if: ${{ steps.cache-node_modules.outputs.cache-hit != 'true' }}
Expand Down
1 change: 1 addition & 0 deletions .npmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
install-strategy = "linked"

@ciampo ciampo Sep 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

npm 11.16 is still allowed, but its linked installation leaves webpack unable to find the installed webpack-cli and breaks the documented webpack command. Could we raise both npm minimum entries to >=11.18.0 and verify that minimum in CI?

@manzoorwanijk manzoorwanijk Sep 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, that makes sense. The expectation, though, is that everyone uses the latest npm version, which the CI already does.

@manzoorwanijk manzoorwanijk Sep 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in f4d31be

@ciampo ciampo Oct 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We're technically not checking on CI that the min required version works well (since it installs latest version), but I think that's ok and not a blocker

Comment thread
manzoorwanijk marked this conversation as resolved.
engine-strict = true
strict-peer-deps = true
prefer-dedupe = true
Expand Down
6 changes: 6 additions & 0 deletions docs/contributors/code/getting-started-with-code-contribution.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,12 @@ You can start with this workspace settings file:
"**/node_modules/**": true,
"**/vendor/**": true
},
// Use TypeScript from the workspace instead of VS Code's built-in TypeScript version.
"js/ts.tsdk.path": "tools/build-scripts/node_modules/@typescript/native/lib",
// Use ESLint, Prettier and Stylelint from the respective paths.
"eslint.nodePath": "tools/eslint/node_modules",
"prettier.prettierPath": "packages/prettier-config/node_modules/prettier",
"stylelint.stylelintPath": "tools/stylelint/node_modules/stylelint",
"[php]": {
"editor.formatOnSave": true,
"editor.defaultFormatter": "obliviousharmony.vscode-php-codesniffer"
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
},
"engines": {
"node": ">=24.18.0",
"npm": ">=11.16.0"
"npm": ">=11.18.0"
},
"devEngines": {
"runtime": {
Expand All @@ -25,7 +25,7 @@
},
"packageManager": {
"name": "npm",
"version": ">=11.16.0",
"version": ">=11.18.0",
"onFail": "error"
}
},
Expand Down
8 changes: 4 additions & 4 deletions packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ exports[`DependencyExtractionWebpackPlugin modules > Webpack \`no-default\` > sh
'dependencies' => array(

),
'version' => 'ef50aa650dbfb7d400af',
'version' => 'e92a9548b4a6bd2acd81',
'type' => 'module'
);
"
Expand Down Expand Up @@ -418,7 +418,7 @@ exports[`DependencyExtractionWebpackPlugin modules > Webpack \`runtime-chunk-sin
'dependencies' => array(
'@wordpress/blob'
),
'version' => '5822b2168d9a9ad0ab52',
'version' => '0b4be29a9a428b9e8608',
'type' => 'module',
'handle' => 'runtime-chunk-single-modules-a'
);
Expand Down Expand Up @@ -858,7 +858,7 @@ exports[`DependencyExtractionWebpackPlugin scripts > Webpack \`no-default\` > sh
'dependencies' => array(

),
'version' => '93d83bee525cbfa3ca27'
'version' => '9c20c9bd8e5f5841767b'
);
"
`;
Expand Down Expand Up @@ -1029,7 +1029,7 @@ exports[`DependencyExtractionWebpackPlugin scripts > Webpack \`runtime-chunk-sin
'dependencies' => array(
'wp-blob'
),
'version' => '10219f2e878437d6563a',
'version' => 'c4233136b50b85f54397',
'handle' => 'runtime-chunk-single-scripts-a'
);
"
Expand Down
14 changes: 13 additions & 1 deletion storybook/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ const config: StorybookConfig = {
// Should match defaults in Storybook except for the propFilter.
// https://github.com/storybookjs/storybook/blob/3e34a288c8fabc7d5b5cc43b28ae9d674c48e3ea/code/core/src/core-server/presets/common-preset.ts#L162-L168
reactDocgenTypescriptOptions: {
/*
* Vite's root is this workspace, so the plugin's default glob stops
* at its edge and the documented sources need naming explicitly.
*/
include: [ '**/*.tsx', '../packages/*/src/**/*.tsx' ],
exclude: [ '**/test/**', '**/node_modules/**' ],
// Use a docgen-specific TypeScript configuration that disables
// project references. Without this, docgen follows referenced
// projects' built `.d.ts` declarations and emits a duplicate
Expand Down Expand Up @@ -130,8 +136,14 @@ const config: StorybookConfig = {
savePropValueAsString: true,
},
},
viteFinal: async ( viteConfig ) => {
viteFinal: async ( viteConfig, options ) => {
return mergeConfig( viteConfig, {
/*
* Storybook roots Vite at the repository root, which owns none of
* the dependencies. Each config roots itself, so the one that
* spreads this gets its own workspace rather than this one.
*/
root: options.configDir,
resolve: {
alias: [
{
Expand Down
10 changes: 5 additions & 5 deletions test/storybook-playwright/storybook/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export default {
...baseConfig.typescript,
reactDocgenTypescriptOptions: {
...baseConfig.typescript?.reactDocgenTypescriptOptions,
// Vite's root is this workspace, so include the source files used
// by these stories outside that root in the docgen project.
// Vite's root is this config's directory, so include the sources
// these stories document from outside it in the docgen project.
include: [
'**/*.tsx',
'../../packages/components/src/**/*.tsx',
'../../packages/ui/src/**/*.tsx',
'../../storybook/**/*.tsx',
'../../../packages/components/src/**/*.tsx',
'../../../packages/ui/src/**/*.tsx',
'../../../storybook/**/*.tsx',
],
},
},
Expand Down
14 changes: 4 additions & 10 deletions tools/monorepo/tsconfig/tsconfig.base.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,11 @@
"resolveJsonModule": true,

/*
* `../typings` is this workspace's own. `@types` packages are not:
* a hoisted install puts them at the repo root, a linked one puts
* them in the workspace that declared them. Both are listed because
* TypeScript skips a type root that does not exist, so the preset
* serves either layout.
* `../typings` is this workspace's own. `@types` packages resolve from
* the project that declared them, so a package using types it never
* declared fails here instead of borrowing them from the root.
*/
"typeRoots": [
"../typings",
"${configDir}/node_modules/@types",
"../../../node_modules/@types"
],
"typeRoots": [ "../typings", "${configDir}/node_modules/@types" ],
"types": [],

"rootDir": "${configDir}/src",
Expand Down
47 changes: 47 additions & 0 deletions tools/validation/check-installed-deps.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Exits non-zero with a hint to run `npm install` if the trees diverge.
*/

import { execFileSync } from 'node:child_process';
import fs from 'node:fs';
import path from 'node:path';
import os from 'node:os';
Expand All @@ -24,6 +25,7 @@ const CACHE_FILE = path.join(
'node_modules',
'.check-installed-deps.cache.json'
);
const STORE = path.join( ROOT, 'node_modules', '.store' );

const verbose = process.argv.includes( '--verbose' );

Expand All @@ -36,6 +38,51 @@ function fail( summary, details = '' ) {
process.exit( 1 );
}

/**
* Read the configured install strategy. npm resolves it across every .npmrc,
* the environment and the command line, so ask npm rather than parse a file.
*
* @return {string} The configured strategy.
*/
function configuredStrategy() {
// Set for anything npm runs, which is how this check is invoked.
if ( process.env.npm_config_install_strategy ) {
return process.env.npm_config_install_strategy;
}

try {
return execFileSync( 'npm', [ 'config', 'get', 'install-strategy' ], {
cwd: ROOT,
encoding: 'utf8',
} ).trim();
} catch {
return 'hoisted';
}
}

/*
* The lockfile is the same under either layout, and the comparison below
* reduces paths to name@version, so a tree installed with the other strategy
* passes everything. Compare the configured strategy against the real tree
* first, and before the mtime fast path, which such a tree would satisfy.
*/
const strategy = configuredStrategy();
const installedIsolated = fs.existsSync( STORE );

if ( strategy === 'linked' && ! installedIsolated ) {
fail(
'npm is configured for the isolated layout but node_modules is hoisted.',
`\tno ${ path.relative( ROOT, STORE ) }; run \`npm install\` to reinstall.`
);
}

if ( strategy !== 'linked' && installedIsolated ) {
fail(
`npm is configured for the ${ strategy } layout but node_modules is isolated.`,
`\t${ path.relative( ROOT, STORE ) } exists; run \`npm install\` to reinstall.`
);
}

/*
* Fast path: skip the full check if neither lockfile has changed since the
* last successful run. Both files' mtimes are written into a cache file
Expand Down
Loading