Skip to content

Commit d565bbe

Browse files
manzoorwanijkciampotyxla
authored and
widoz
committed
Framework: Switch from hoisted to isolated dependencies (WordPress#83847)
* Framework: Switch to npm install-strategy=linked Dependencies are installed into node_modules/.store/ and symlinked to the packages that declare them, so a package can no longer resolve a dependency it never declared. * Framework: Adapt tooling to the isolated install layout typeRoots drops its repo-root @types fallback, so a package using types it never declared fails instead of borrowing them. Jest transforms through .store/, and test-create-block gives wp-scripts a NODE_PATH fallback for the bins it shells out to. Storybook roots Vite at the repository root by default, which under the isolated layout owns none of the dependencies its smoke tests resolve. CI skips the node_modules cache on Windows, whose tar cannot restore the isolated layout's links. * Framework: Require npm v11.18 The linked install strategy is stable from v11.18, so the engine range no longer accepts the versions that shipped it as experimental. * Build: Detect a node_modules installed with the other strategy The lockfile is identical under either layout and the installed-deps check reduces paths to name@version, so a tree installed with the other strategy satisfied every comparison and the mtime fast path skipped the check entirely. Compare npm's configured install-strategy against the tree on disk first, so switching strategies without a lockfile change still asks for a reinstall. * Storybook: Restore docgen coverage under the isolated layout Rooting Vite at the Storybook workspace left the docgen plugin's default glob stopping at that root, so prop tables covered 298 components instead of 1079. Naming the documented sources restores them; excluding stories would drop the components they define. The root now derives from each config's own directory, so the Playwright config no longer inherits this workspace as its root. Its docgen globs move with it, matching the story globs beside them. * Framework: Drop the NODE_PATH fallback in create-block validation The fallback worked around `@wordpress/scripts` resolving its default browserslist config from the consumer's cwd, which WordPress#83902 fixes in the package itself by passing the preset's queries to webpack. Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org>
1 parent d08a0c6 commit d565bbe

10 files changed

Lines changed: 88 additions & 25 deletions

File tree

‎.github/setup-node/action.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,14 @@ runs:
3232
echo "NPM_VERSION=$(npm -v)" >> "$GITHUB_OUTPUT"
3333
shell: bash
3434

35+
# Windows tar cannot restore the isolated layout's links, so always install there.
3536
- name: Cache node_modules
3637
id: cache-node_modules
37-
if: ${{ inputs.cache == 'true' }}
38+
if: ${{ inputs.cache == 'true' && runner.os != 'Windows' }}
3839
uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
3940
with:
41+
# The key covers .npmrc because install-strategy decides the layout:
42+
# a hoisted tree restored into an isolated checkout resolves nothing.
4043
# List workspace node_modules explicitly to support all kinds of layouts - hoisted and isolated.
4144
# A '**/node_modules' glob follows the isolated layout's symlinks and stalls the cache step.
4245
# The eval package is not a workspace: it installs its own large tree from its own lockfile.
@@ -49,7 +52,7 @@ runs:
4952
!test/ai-development/node_modules
5053
tools/*/node_modules
5154
widgets/*/node_modules
52-
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/**') }}
55+
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') }}
5356

5457
- name: Install npm dependencies
5558
if: ${{ steps.cache-node_modules.outputs.cache-hit != 'true' }}

‎.npmrc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
install-strategy = "linked"
12
engine-strict = true
23
strict-peer-deps = true
34
prefer-dedupe = true

‎docs/contributors/code/getting-started-with-code-contribution.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,12 @@ You can start with this workspace settings file:
241241
"**/node_modules/**": true,
242242
"**/vendor/**": true
243243
},
244+
// Use TypeScript from the workspace instead of VS Code's built-in TypeScript version.
245+
"js/ts.tsdk.path": "tools/build-scripts/node_modules/@typescript/native/lib",
246+
// Use ESLint, Prettier and Stylelint from the respective paths.
247+
"eslint.nodePath": "tools/eslint/node_modules",
248+
"prettier.prettierPath": "packages/prettier-config/node_modules/prettier",
249+
"stylelint.stylelintPath": "tools/stylelint/node_modules/stylelint",
244250
"[php]": {
245251
"editor.formatOnSave": true,
246252
"editor.defaultFormatter": "obliviousharmony.vscode-php-codesniffer"

‎package-lock.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
},
1717
"engines": {
1818
"node": ">=24.18.0",
19-
"npm": ">=11.16.0"
19+
"npm": ">=11.18.0"
2020
},
2121
"devEngines": {
2222
"runtime": {
@@ -25,7 +25,7 @@
2525
},
2626
"packageManager": {
2727
"name": "npm",
28-
"version": ">=11.16.0",
28+
"version": ">=11.18.0",
2929
"onFail": "error"
3030
}
3131
},

‎packages/dependency-extraction-webpack-plugin/test/__snapshots__/build.js.snap‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ exports[`DependencyExtractionWebpackPlugin modules > Webpack \`no-default\` > sh
253253
'dependencies' => array(
254254
255255
),
256-
'version' => 'ef50aa650dbfb7d400af',
256+
'version' => 'e92a9548b4a6bd2acd81',
257257
'type' => 'module'
258258
);
259259
"
@@ -418,7 +418,7 @@ exports[`DependencyExtractionWebpackPlugin modules > Webpack \`runtime-chunk-sin
418418
'dependencies' => array(
419419
'@wordpress/blob'
420420
),
421-
'version' => '5822b2168d9a9ad0ab52',
421+
'version' => '0b4be29a9a428b9e8608',
422422
'type' => 'module',
423423
'handle' => 'runtime-chunk-single-modules-a'
424424
);
@@ -858,7 +858,7 @@ exports[`DependencyExtractionWebpackPlugin scripts > Webpack \`no-default\` > sh
858858
'dependencies' => array(
859859
860860
),
861-
'version' => '93d83bee525cbfa3ca27'
861+
'version' => '9c20c9bd8e5f5841767b'
862862
);
863863
"
864864
`;
@@ -1029,7 +1029,7 @@ exports[`DependencyExtractionWebpackPlugin scripts > Webpack \`runtime-chunk-sin
10291029
'dependencies' => array(
10301030
'wp-blob'
10311031
),
1032-
'version' => '10219f2e878437d6563a',
1032+
'version' => 'c4233136b50b85f54397',
10331033
'handle' => 'runtime-chunk-single-scripts-a'
10341034
);
10351035
"

‎storybook/main.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ const config: StorybookConfig = {
9494
// Should match defaults in Storybook except for the propFilter.
9595
// https://github.com/storybookjs/storybook/blob/3e34a288c8fabc7d5b5cc43b28ae9d674c48e3ea/code/core/src/core-server/presets/common-preset.ts#L162-L168
9696
reactDocgenTypescriptOptions: {
97+
/*
98+
* Vite's root is this workspace, so the plugin's default glob stops
99+
* at its edge and the documented sources need naming explicitly.
100+
*/
101+
include: [ '**/*.tsx', '../packages/*/src/**/*.tsx' ],
102+
exclude: [ '**/test/**', '**/node_modules/**' ],
97103
// Use a docgen-specific TypeScript configuration that disables
98104
// project references. Without this, docgen follows referenced
99105
// projects' built `.d.ts` declarations and emits a duplicate
@@ -130,8 +136,14 @@ const config: StorybookConfig = {
130136
savePropValueAsString: true,
131137
},
132138
},
133-
viteFinal: async ( viteConfig ) => {
139+
viteFinal: async ( viteConfig, options ) => {
134140
return mergeConfig( viteConfig, {
141+
/*
142+
* Storybook roots Vite at the repository root, which owns none of
143+
* the dependencies. Each config roots itself, so the one that
144+
* spreads this gets its own workspace rather than this one.
145+
*/
146+
root: options.configDir,
135147
resolve: {
136148
alias: [
137149
{

‎test/storybook-playwright/storybook/main.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ export default {
99
...baseConfig.typescript,
1010
reactDocgenTypescriptOptions: {
1111
...baseConfig.typescript?.reactDocgenTypescriptOptions,
12-
// Vite's root is this workspace, so include the source files used
13-
// by these stories outside that root in the docgen project.
12+
// Vite's root is this config's directory, so include the sources
13+
// these stories document from outside it in the docgen project.
1414
include: [
1515
'**/*.tsx',
16-
'../../packages/components/src/**/*.tsx',
17-
'../../packages/ui/src/**/*.tsx',
18-
'../../storybook/**/*.tsx',
16+
'../../../packages/components/src/**/*.tsx',
17+
'../../../packages/ui/src/**/*.tsx',
18+
'../../../storybook/**/*.tsx',
1919
],
2020
},
2121
},

‎tools/monorepo/tsconfig/tsconfig.base.json‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,11 @@
4343
"resolveJsonModule": true,
4444

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

5953
"rootDir": "${configDir}/src",

‎tools/validation/check-installed-deps.mjs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* Exits non-zero with a hint to run `npm install` if the trees diverge.
1111
*/
1212

13+
import { execFileSync } from 'node:child_process';
1314
import fs from 'node:fs';
1415
import path from 'node:path';
1516
import os from 'node:os';
@@ -24,6 +25,7 @@ const CACHE_FILE = path.join(
2425
'node_modules',
2526
'.check-installed-deps.cache.json'
2627
);
28+
const STORE = path.join( ROOT, 'node_modules', '.store' );
2729

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

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

41+
/**
42+
* Read the configured install strategy. npm resolves it across every .npmrc,
43+
* the environment and the command line, so ask npm rather than parse a file.
44+
*
45+
* @return {string} The configured strategy.
46+
*/
47+
function configuredStrategy() {
48+
// Set for anything npm runs, which is how this check is invoked.
49+
if ( process.env.npm_config_install_strategy ) {
50+
return process.env.npm_config_install_strategy;
51+
}
52+
53+
try {
54+
return execFileSync( 'npm', [ 'config', 'get', 'install-strategy' ], {
55+
cwd: ROOT,
56+
encoding: 'utf8',
57+
} ).trim();
58+
} catch {
59+
return 'hoisted';
60+
}
61+
}
62+
63+
/*
64+
* The lockfile is the same under either layout, and the comparison below
65+
* reduces paths to name@version, so a tree installed with the other strategy
66+
* passes everything. Compare the configured strategy against the real tree
67+
* first, and before the mtime fast path, which such a tree would satisfy.
68+
*/
69+
const strategy = configuredStrategy();
70+
const installedIsolated = fs.existsSync( STORE );
71+
72+
if ( strategy === 'linked' && ! installedIsolated ) {
73+
fail(
74+
'npm is configured for the isolated layout but node_modules is hoisted.',
75+
`\tno ${ path.relative( ROOT, STORE ) }; run \`npm install\` to reinstall.`
76+
);
77+
}
78+
79+
if ( strategy !== 'linked' && installedIsolated ) {
80+
fail(
81+
`npm is configured for the ${ strategy } layout but node_modules is isolated.`,
82+
`\t${ path.relative( ROOT, STORE ) } exists; run \`npm install\` to reinstall.`
83+
);
84+
}
85+
3986
/*
4087
* Fast path: skip the full check if neither lockfile has changed since the
4188
* last successful run. Both files' mtimes are written into a cache file

0 commit comments

Comments
 (0)