Skip to content

Commit 254f154

Browse files
committed
fix: pass pluginsEnv to integration build hooks build step
For context: `@netlify/build` automatically builds integrations when `context` is set to `dev`. This PR targets that functionality. Without this change, `buildSite`'s programmatic interface doesn't pass programatically defined `env` variables into the task that builds ~integrations~ extensions' build hooks. ```ts import buildSite from "@netlify/build"; await buildSite({ env: { // This is passed to the build plugin when it's executed, but is not // passed to the task that builds the plugin. SOME_ENV_VAR: "1", }, }); ``` I don't know if `pluginsEnv` or `childEnv` is the more appropriate value to pass here, but `pluginsEnv` seems more consistent with how build treats integrations as plugins, so I chose to use it. Two total digressions I thought of while writing this: - We should probably be passing `extendEnv: false` to the `execa` invocation that builds the integration, but I'm not sure if that would break something at this point, so I'm leaving it as is for now. - I sort of hate that `@netlify/build` builds the integration automatically. It's challenging to use it in tests: every test rebuilds the integration (which is expensive) and you have to make sure that no two tests that auto-build the integration build it concurrently, otherwise you might end up with test flakes when one test writes to the built tarball while another test is installing from it. I don't yet know what the best way to solve this problem is, so I'm punting on solving it for now.
1 parent 198329a commit 254f154

File tree

4 files changed

+16
-10
lines changed

4 files changed

+16
-10
lines changed

packages/build/src/core/build.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,11 @@ const initAndRunBuild = async function ({
449449
edgeFunctionsBootstrapURL,
450450
eventHandlers,
451451
}) {
452+
const pluginsEnv = {
453+
...childEnv,
454+
...getBlobsEnvironmentContext({ api, deployId: deployId, siteId: siteInfo?.id, token }),
455+
}
456+
452457
const { pluginsOptions: pluginsOptionsA, timers: timersA } = await getPluginsOptions({
453458
pluginsOptions,
454459
netlifyConfig,
@@ -469,13 +474,9 @@ const initAndRunBuild = async function ({
469474
integrations,
470475
context,
471476
systemLog,
477+
pluginsEnv,
472478
})
473479

474-
const pluginsEnv = {
475-
...childEnv,
476-
...getBlobsEnvironmentContext({ api, deployId: deployId, siteId: siteInfo?.id, token }),
477-
}
478-
479480
if (pluginsOptionsA?.length) {
480481
const buildPlugins = {}
481482
for (const plugin of pluginsOptionsA) {

packages/build/src/install/missing.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const installIntegrationPlugins = async function ({
3434
logs,
3535
context,
3636
testOpts,
37+
pluginsEnv,
3738
}) {
3839
const integrationsToBuild = integrations.filter(
3940
(integration) => typeof integration.dev !== 'undefined' && context === 'dev',
@@ -46,7 +47,7 @@ export const installIntegrationPlugins = async function ({
4647
)
4748
}
4849
const packages = (
49-
await Promise.all(integrations.map((integration) => getIntegrationPackage({ integration, context, testOpts })))
50+
await Promise.all(integrations.map((integration) => getIntegrationPackage({ integration, context, testOpts, pluginsEnv })))
5051
).filter(Boolean)
5152
logInstallIntegrations(
5253
logs,
@@ -64,7 +65,7 @@ export const installIntegrationPlugins = async function ({
6465
await addExactDependencies({ packageRoot: autoPluginsDir, isLocal: mode !== 'buildbot', packages })
6566
}
6667

67-
const getIntegrationPackage = async function ({ integration: { version, dev }, context, testOpts = {} }) {
68+
const getIntegrationPackage = async function ({ integration: { version, dev }, context, testOpts = {}, pluginsEnv }) {
6869
if (typeof version !== 'undefined') {
6970
return `${version}/packages/buildhooks.tgz`
7071
}
@@ -74,7 +75,7 @@ const getIntegrationPackage = async function ({ integration: { version, dev }, c
7475

7576
const integrationDir = testOpts.cwd ? resolve(testOpts.cwd, path) : resolve(path)
7677
try {
77-
const res = await execa('npm', ['run', 'build'], { cwd: integrationDir })
78+
const res = await execa('npm', ['run', 'build'], { cwd: integrationDir, env: pluginsEnv })
7879

7980
// This is horrible and hacky, but `npm run build` will
8081
// return status code 0 even if it fails

packages/build/src/plugins/options.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ const tGetPluginsOptions = async function ({
3232
integrations,
3333
context,
3434
systemLog,
35+
pluginsEnv,
3536
}) {
3637
const pluginsOptionsA = await resolvePluginsPath({
3738
pluginsOptions,
@@ -51,6 +52,7 @@ const tGetPluginsOptions = async function ({
5152
integrations,
5253
context,
5354
systemLog,
55+
pluginsEnv,
5456
})
5557
const pluginsOptionsB = await Promise.all(
5658
pluginsOptionsA.map((pluginOptions) => loadPluginFiles({ pluginOptions, debug })),

packages/build/src/plugins/resolve.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export const resolvePluginsPath = async function ({
3333
integrations,
3434
context,
3535
systemLog,
36+
pluginsEnv,
3637
}) {
3738
const autoPluginsDir = getAutoPluginsDir(buildDir, packagePath)
3839
const pluginsOptionsA = await Promise.all(
@@ -77,6 +78,7 @@ export const resolvePluginsPath = async function ({
7778
buildDir,
7879
context,
7980
testOpts,
81+
pluginsEnv,
8082
})
8183

8284
return [...pluginsOptionsE, ...integrationPluginOptions]
@@ -164,9 +166,9 @@ const handleMissingPlugins = async function ({ pluginsOptions, autoPluginsDir, m
164166
return Promise.all(pluginsOptions.map((pluginOptions) => resolveMissingPluginPath({ pluginOptions, autoPluginsDir })))
165167
}
166168

167-
const handleIntegrations = async function ({ integrations, autoPluginsDir, mode, logs, buildDir, context, testOpts }) {
169+
const handleIntegrations = async function ({ integrations, autoPluginsDir, mode, logs, buildDir, context, testOpts, pluginsEnv }) {
168170
const toInstall = integrations.filter((integration) => integration.has_build)
169-
await installIntegrationPlugins({ integrations: toInstall, autoPluginsDir, mode, logs, context, testOpts })
171+
await installIntegrationPlugins({ integrations: toInstall, autoPluginsDir, mode, logs, context, testOpts, pluginsEnv })
170172

171173
if (toInstall.length === 0) {
172174
return []

0 commit comments

Comments
 (0)