Skip to content

fix: resolve site dependency version from packagePath if defined #6650

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
merged 4 commits into from
Sep 4, 2025
Merged

fix: resolve site dependency version from packagePath if defined #6650

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
579c160
test: add cases for monorepo and build plugins with siteDependencies …
pieh Sep 4, 2025
5a01fdc
fix: resolve site dependency version from packagePath if defined
pieh Sep 4, 2025
a45722f
chore: revert some unnecesary test helper changes
pieh Sep 4, 2025
596e0b2
chore: instead of ternary use join with fallback as we use in other p…
pieh Sep 4, 2025
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
9 changes: 7 additions & 2 deletions packages/build/src/plugins/plugin_conditions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const siteDependenciesTest = async function (
return (
await Promise.all(
Object.entries(allowedSiteDependencies).map(async ([dependencyName, allowedVersion]) =>
siteDependencyTest({ dependencyName, allowedVersion, siteDependencies, buildDir }),
siteDependencyTest({ dependencyName, allowedVersion, siteDependencies, buildDir, packagePath }),
),
)
).every(Boolean)
Expand All @@ -63,10 +63,12 @@ const siteDependencyTest = async function ({
allowedVersion,
siteDependencies,
buildDir,
packagePath,
}: {
dependencyName: string
allowedVersion: string
buildDir: string
packagePath?: string
siteDependencies: Record<string, string | undefined>
}): Promise<boolean> {
const siteDependency = siteDependencies[dependencyName]
Expand All @@ -81,7 +83,10 @@ const siteDependencyTest = async function ({

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`,
packagePath ? join(buildDir, packagePath) : buildDir,
Copy link
Contributor

@mrstork mrstork Sep 4, 2025

Choose a reason for hiding this comment

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

nit - we seem to mostly be using join(buildDir, packagePath || '') or packagePath ?? ''

)
const { version } = await importJsonFile(packageJsonPath)
if (!version) {
return false
Expand Down
2 changes: 2 additions & 0 deletions ...st/fixtures/plugins_compat_site_dependencies_range_monorepo_hoisted/apps/web/netlify.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "netlify-plugin-contextual-env"
11 changes: 11 additions & 0 deletions ...st/fixtures/plugins_compat_site_dependencies_range_monorepo_hoisted/apps/web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "module_plugin",
"version": "0.0.1",
"type": "module",
"description": "test",
"license": "MIT",
"repository": "test",
"dependencies": {
"@netlify/dependency-with-range": "^9.5.4"
}
}
3 changes: 3 additions & 0 deletions ...endencies_range_monorepo_hoisted/node_modules/@netlify/dependency-with-range/package.json
View file
Open in desktop

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

5 changes: 5 additions & 0 deletions ...lugins_list/fixtures/plugins_compat_site_dependencies_range_monorepo_hoisted/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"workspaces": [
"apps/*"
]
}
2 changes: 2 additions & 0 deletions ...ixtures/plugins_compat_site_dependencies_range_monorepo_not_hoisted/apps/web/netlify.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "netlify-plugin-contextual-env"
3 changes: 3 additions & 0 deletions ...ge_monorepo_not_hoisted/apps/web/node_modules/@netlify/dependency-with-range/package.json
View file
Open in desktop

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

11 changes: 11 additions & 0 deletions ...ixtures/plugins_compat_site_dependencies_range_monorepo_not_hoisted/apps/web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "module_plugin",
"version": "0.0.1",
"type": "module",
"description": "test",
"license": "MIT",
"repository": "test",
"dependencies": {
"@netlify/dependency-with-range": "^9.5.4"
}
}
5 changes: 5 additions & 0 deletions ...ns_list/fixtures/plugins_compat_site_dependencies_range_monorepo_not_hoisted/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"workspaces": [
"apps/*"
]
}
74 changes: 71 additions & 3 deletions packages/build/tests/plugins_list/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import cpy from 'cpy'

const FIXTURES_DIR = fileURLToPath(new URL('fixtures', import.meta.url))

const runWithApiMock = async function (
t,
const runWithApiMockAndGetNormalizedOutput = async function (
fixtureName,
{ testPlugin, response = getPluginsList(testPlugin), ...flags } = {},
status = 200,
Expand All @@ -26,12 +25,21 @@ const runWithApiMock = async function (
...flags,
})
.runWithBuild()
await t.snapshot(normalizeOutput(output))
return normalizeOutput(output)
} finally {
await stopServer()
}
}

const runWithApiMock = async function (
t,
fixtureName,
{ testPlugin, response = getPluginsList(testPlugin), ...flags } = {},
status = 200,
) {
await t.snapshot(await runWithApiMockAndGetNormalizedOutput(fixtureName, { testPlugin, response, ...flags }, status))
}

Comment on lines +34 to +42
Copy link
Contributor Author

@pieh pieh Sep 4, 2025

Choose a reason for hiding this comment

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

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

// We use a specific plugin in tests. We hardcode its version to keep the tests
// stable even when new versions of that plugin are published.
const getPluginsList = function (testPlugin = DEFAULT_TEST_PLUGIN) {
Expand Down Expand Up @@ -289,6 +297,66 @@ test.serial('Plugins can specify non-matching compatibility.siteDependencies ran
})
})

test.serial(
'Plugins can specify matching compatibility.siteDependencies range in monorepo with hoisted node_modules',
async (t) => {
await removeDir(`${FIXTURES_DIR}/plugins_compat_site_dependencies_range_monorepo_hoisted/apps/web/.netlify`)
const normalizedOutput = await runWithApiMockAndGetNormalizedOutput(
'plugins_compat_site_dependencies_range_monorepo_hoisted',
{
testPlugin: {
compatibility: [
{ version: '0.3.0' },
{
version: '0.2.0',
siteDependencies: {
// this is satisfied, so this version should be selected
'@netlify/dependency-with-range': '<10',
},
},
],
},
packagePath: 'apps/web',
},
)
t.true(
normalizedOutput.includes(
'netlify-plugin-contextual-env 0-2-0 from netlify.toml (latest 0-3-0, expected 0-2-0, compatible 0-2-0)',
),
)
},
)

test.serial(
'Plugins can specify matching compatibility.siteDependencies range in monorepo without hoisted node_modules',
async (t) => {
await removeDir(`${FIXTURES_DIR}/plugins_compat_site_dependencies_range_monorepo_not_hoisted/apps/web/.netlify`)
const normalizedOutput = await runWithApiMockAndGetNormalizedOutput(
'plugins_compat_site_dependencies_range_monorepo_not_hoisted',
{
testPlugin: {
compatibility: [
{ version: '0.3.0' },
{
version: '0.2.0',
siteDependencies: {
// this is satisfied, so this version should be selected
'@netlify/dependency-with-range': '<10',
},
},
],
},
packagePath: 'apps/web',
},
)
t.true(
normalizedOutput.includes(
'netlify-plugin-contextual-env 0-2-0 from netlify.toml (latest 0-3-0, expected 0-2-0, compatible 0-2-0)',
),
)
},
)

test.serial('Plugin versions can be feature flagged', async (t) => {
await removeDir(`${FIXTURES_DIR}/plugins_compat_node_version/.netlify`)
await runWithApiMock(t, 'plugins_compat_node_version', {
Expand Down
Loading