Skip to content

Commit b31aa6a

Browse files
authored
feat(build,config)!: rework extension development workflow (#6571)
This changeset makes a light breaking change to the way local extension build plugins work. This fixes an issue where functions injected using the `functions.generate` API silently fail to include `node_modules` required for the function to work. Currently, during `netlify build` and `netlify dev`, build will automatically build your extension and include any build plugins produced by that extension in the build process if you specify this configuration in your `netlify.toml` (documented [here](https://developers.netlify.com/sdk/build-event-handlers/debug-and-test/)): ```toml # netlify.toml [[integrations]] name = "my-local-extension" [[integrations.dev]] path = "../some-local-path-to-integration" force_run_in_build = true ``` To make it short(ish): This process makes a lot of assumptions about the default file structure of a Netlify Extension. It also installs dev-mode extension build plugins using a different code path than for production extension build plugins: In production, we do an `npm install` against a tarball; in development, we point build at a directory and no installation is performed. (This is the crux of the `functions.generate` problem: the directory we point build at is a temporary build directory and does not contain `node_modules`.) We use dev-mode extensions in SDK/build integration tests, so this problem breaks our existing tests and generally makes it difficult to trust that SDK tests will catch real-world, user-facing build plugin problems. With these changes, build no longer automatically builds extensions. Instead, it assumes that you've already built your extension, and will now install extension build plugins from a local tarball. This makes dev mode behave very similarly to production mode builds. You can now point build at a tarball: ```toml # netlify.toml [[integrations]] name = "my-local-extension" [[integrations.dev]] path = "../some-local-path-to-integration/buildhooks.tgz" # Note: force_run_in_build is no longer required ``` To make the transition on this change easier, you can instead point build at a directory, and the historical default extension build hooks tarball path will be inferred: ```toml # netlify.toml [[integrations]] name = "my-local-extension" [[integrations.dev]] # Expands to "../some-local-path-to-integration/.ntli/site/static/packages/buildhooks.tgz" path = "../some-local-path-to-integration" ``` There's part of me that thinks, "This behaves slightly differently than local build plugins, and that asymmetry sucks." But: Local build plugins are meant to work when developed in the same tree as the site they're modifying. For better or for worse, developing extensions in the same tree as a site is not supported. So, this asymmetry actually feels appropriate to me. --- Footnote: This corner of the code had a lot of dead code, code that was untyped, code that was added to avoid updating test fixtures, code that was unnecessarily complex, etc. etc. I did some refactoring along the way to make this functionality easier to implement, and those refactors are included in this PR. Most of the substantial stuff is concentrated in 5-6 files and is fairly well split out into separate commits. To weed out refactors from feature work, though, you might find this easier to review commit by commit.
1 parent 56b91ce commit b31aa6a

File tree

37 files changed

+660
-406
lines changed

37 files changed

+660
-406
lines changed

package-lock.json

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

packages/build/src/install/missing.js

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
import { promises as fs } from 'fs'
2-
import { normalize, resolve } from 'path'
1+
import { promises as fs } from 'node:fs'
2+
import { normalize } from 'node:path'
3+
import { fileURLToPath } from 'node:url'
34

4-
import { execa } from 'execa'
55
import { pathExists } from 'path-exists'
66
import { isFile } from 'path-type'
77

8-
import { logArray, logSubHeader } from '../log/logger.js'
98
import { logInstallMissingPlugins, logInstallIntegrations } from '../log/messages/install.js'
109

1110
import { addExactDependencies } from './main.js'
@@ -37,71 +36,38 @@ export const installIntegrationPlugins = async function ({
3736
pluginsEnv,
3837
buildDir,
3938
}) {
40-
const integrationsToBuild = integrations.filter(
41-
(integration) => typeof integration.dev !== 'undefined' && context === 'dev',
42-
)
43-
if (integrationsToBuild.length) {
44-
logSubHeader(logs, 'Building extensions')
45-
logArray(
46-
logs,
47-
integrationsToBuild.map(({ slug, dev: { path } }) => `${slug} from ${path}`),
48-
)
49-
}
5039
const packages = (
5140
await Promise.all(
5241
integrations.map((integration) =>
5342
getIntegrationPackage({ integration, context, testOpts, buildDir, pluginsEnv }),
5443
),
5544
)
5645
).filter(Boolean)
57-
logInstallIntegrations(
58-
logs,
59-
integrations.filter((integration) =>
60-
integrationsToBuild.every((compiledIntegration) => integration.slug !== compiledIntegration.slug),
61-
),
62-
)
46+
logInstallIntegrations(logs, integrations)
6347

6448
if (packages.length === 0) {
6549
return
6650
}
6751

6852
await createAutoPluginsDir(logs, autoPluginsDir)
69-
7053
await addExactDependencies({ packageRoot: autoPluginsDir, isLocal: mode !== 'buildbot', packages })
7154
}
7255

73-
const getIntegrationPackage = async function ({
74-
integration: { version, dev },
75-
context,
76-
testOpts = {},
77-
buildDir,
78-
pluginsEnv,
79-
}) {
80-
if (typeof version !== 'undefined') {
81-
return `${version}/packages/buildhooks.tgz`
82-
}
83-
84-
if (typeof dev !== 'undefined' && context === 'dev') {
85-
const { path } = dev
86-
87-
const integrationDir = testOpts.cwd ? resolve(testOpts.cwd, path) : resolve(buildDir, path)
88-
89-
try {
90-
const res = await execa('npm', ['run', 'build'], { cwd: integrationDir, env: pluginsEnv })
91-
92-
// This is horrible and hacky, but `npm run build` will
93-
// return status code 0 even if it fails
94-
if (!res.stdout.includes('Build complete!')) {
95-
throw new Error(res.stdout)
96-
}
97-
} catch (e) {
98-
throw new Error(`Failed to build integration. Error:\n\n${e.stack}`)
99-
}
100-
56+
const getIntegrationPackage = async function ({ integration: { buildPlugin } }) {
57+
if (buildPlugin === null) {
10158
return undefined
10259
}
10360

104-
return undefined
61+
switch (buildPlugin.packageURL.protocol) {
62+
case 'http:':
63+
// fallthrough
64+
case 'https:':
65+
return buildPlugin.packageURL.toString()
66+
case 'file:':
67+
return fileURLToPath(buildPlugin.packageURL)
68+
default:
69+
throw new Error(`unsupported build plugin package URL: ${buildPlugin.packageURL.toString()}`)
70+
}
10571
}
10672

10773
// We pin the version without using semver ranges ^ nor ~

packages/build/src/plugins/resolve.js

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { join, resolve } from 'path'
1+
import { join } from 'node:path'
22

33
import { addErrorInfo } from '../error/info.js'
44
import { installMissingPlugins, installIntegrationPlugins } from '../install/missing.js'
@@ -66,9 +66,7 @@ export const resolvePluginsPath = async function ({
6666
logs,
6767
})
6868

69-
let integrationPluginOptions = []
70-
71-
integrationPluginOptions = await handleIntegrations({
69+
const integrationPluginOptions = await handleIntegrations({
7270
integrations,
7371
autoPluginsDir,
7472
mode,
@@ -175,9 +173,9 @@ const handleIntegrations = async function ({
175173
testOpts,
176174
pluginsEnv,
177175
}) {
178-
const toInstall = integrations.filter((integration) => integration.has_build)
176+
const integrationsWithBuildPlugins = integrations.filter((integration) => integration.has_build)
179177
await installIntegrationPlugins({
180-
integrations: toInstall,
178+
integrations: integrationsWithBuildPlugins,
181179
autoPluginsDir,
182180
mode,
183181
logs,
@@ -186,38 +184,29 @@ const handleIntegrations = async function ({
186184
buildDir,
187185
pluginsEnv,
188186
})
189-
190-
if (toInstall.length === 0) {
191-
return []
192-
}
193-
194187
return Promise.all(
195-
toInstall.map((integration) =>
196-
resolveIntegration({
188+
integrationsWithBuildPlugins.map(async (integration) => {
189+
// TODO(ndhoule): When developing locally, the user can accidentally set an extension name in
190+
// their netlify.toml that points to a build plugin package that has a mismatched name. This
191+
// will result in a failed install here. The solution is non-obvious: make sure your
192+
// `netlify.toml#integrations[].name` matches the extension that `netlify.toml#integrations[].dev.path`
193+
// points at.
194+
//
195+
// (We could, for example, detect a mismatch by untarring the local build plugin package in
196+
// memory and comparing its `package.json#name` to `integration.slug`, and throw a descriptive
197+
// error if they don't match.)
198+
const packageName = `${integration.slug}-buildhooks`
199+
return {
197200
integration,
198-
autoPluginsDir,
199-
buildDir,
200-
context,
201-
testOpts,
202-
}),
203-
),
201+
isIntegration: true,
202+
loadedFrom: integration.buildPlugin.origin === 'local' ? 'local' : undefined,
203+
packageName,
204+
pluginPath: await resolvePath(packageName, autoPluginsDir),
205+
}
206+
}),
204207
)
205208
}
206209

207-
const resolveIntegration = async function ({ integration, autoPluginsDir, buildDir, context, testOpts }) {
208-
if (typeof integration.dev !== 'undefined' && context === 'dev') {
209-
const { path } = integration.dev
210-
const integrationDir = testOpts.cwd ? resolve(testOpts.cwd, path) : resolve(buildDir, path)
211-
const pluginPath = await resolvePath(`${integrationDir}/.ntli/build`, buildDir)
212-
213-
return { pluginPath, packageName: `${integration.slug}`, isIntegration: true, integration, loadedFrom: 'local' }
214-
}
215-
216-
const pluginPath = await resolvePath(`${integration.slug}-buildhooks`, autoPluginsDir)
217-
218-
return { pluginPath, packageName: `${integration.slug}-buildhooks`, isIntegration: true, integration }
219-
}
220-
221210
// Resolve the plugins that just got automatically installed
222211
const resolveMissingPluginPath = async function ({ pluginOptions, pluginOptions: { packageName }, autoPluginsDir }) {
223212
if (!isMissingPlugin(pluginOptions)) {

packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/index.js

Lines changed: 0 additions & 3 deletions
This file was deleted.

packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/manifest.yml

Lines changed: 0 additions & 2 deletions
This file was deleted.

packages/build/tests/install/fixtures/local_missing_integration/integration/.ntli/build/package.json

Lines changed: 0 additions & 7 deletions
This file was deleted.

packages/build/tests/install/fixtures/local_missing_integration/integration/main.js

Lines changed: 0 additions & 3 deletions
This file was deleted.

packages/build/tests/install/fixtures/local_missing_integration/integration/manifest.yml

Lines changed: 0 additions & 2 deletions
This file was deleted.

packages/build/tests/install/fixtures/local_missing_integration/integration/package.json

Lines changed: 0 additions & 15 deletions
This file was deleted.

0 commit comments

Comments
 (0)