fix: resolve site dependency version from packagePath if defined #6650
Conversation
pieh force-pushed the fix/resolve-site-dependency-version-from-package-path-first branch from 4c22d9e to 9d1f119 Compare 
pieh force-pushed the fix/resolve-site-dependency-version-from-package-path-first branch from 9d1f119 to 579c160 Compare 
| This pull request adds or modifies JavaScript ( |
pieh changed the title 
| const runWithApiMock = async function ( | ||
| t, | ||
| fixtureName, | ||
| { testPlugin, response = getPluginsList(testPlugin), ...flags } = {}, | ||
| status = 200, | ||
| ) { | ||
| await t.snapshot(await runWithApiMockAndGetNormalizedOutput(fixtureName, { testPlugin, response, ...flags }, status)) | ||
| } | ||
| |
There was a problem hiding this comment.
All existing tests in this suite use snapshots which is not something I wanted to continue and wanted something a bit more concrete, so I moved most of this to another helper that just returns normalized output instead of snapshotting it.
Would be nice to convert existing tests to explicit assertions, but I don't have bandwidth for that right now
| const packageJsonPath = await resolvePath(`${dependencyName}/package.json`, buildDir) | ||
| const packageJsonPath = await resolvePath( | ||
| `${dependencyName}/package.json`, | ||
| packagePath ? join(buildDir, packagePath) : buildDir, |
There was a problem hiding this comment.
nit - we seem to mostly be using join(buildDir, packagePath || '') or packagePath ?? ''
| try { | ||
| // if this is a range we need to get the exact version | ||
| const packageJsonPath = await resolvePath(`${dependencyName}/package.json`, buildDir) | ||
| const packageJsonPath = await resolvePath(`${dependencyName}/package.json`, join(buildDir, packagePath || '')) |
There was a problem hiding this comment.
Very similar to fix in #6105, where other part of the system was not taking packagePath into consideration
Do note that if dependency is in node_modules in root of monorepo - we still will be able to resolve, because require.resolve is used and not direct FS check. This is also verified with added test case for hoisted node_modules still passing
…laces in codebase
pieh force-pushed the fix/resolve-site-dependency-version-from-package-path-first branch from fdeca6f to 596e0b2 Compare pieh deleted the fix/resolve-site-dependency-version-from-package-path-first branch 
🎉 Thanks for submitting a pull request! 🎉
Summary
Ref: https://netlify.slack.com/archives/C01TKAEBP3Q/p1756940014614849
To easier follow added fixtures - recommend checking them in "file explorer" instead of diff and notice where
node_modulesare situated in those 2 fixtures:First commit with failing test that this PR is looking to address - https://github.com/netlify/build/actions/runs/17465897278/job/49601431168?pr=6650#step:12:313
Second commit with the fix
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)