From a6f0f45e6453fca74af3bb007fca81a8b7714f86 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 8 Apr 2024 12:31:35 +0200 Subject: [PATCH] feat: add `--internal-disable-edge-functions` flag to skip Deno setup (#6495) * refactor: replace JSDoc types with ts types * feat: make edge functions proxy optional * feat: add --disable-edge-functions flag * fix: tsc errors * chore: write test * chore: be a good citizen and remove a snapshot test * chore: format * feat: add warning line * chore: remove snapshot * fix: format * chore: update docs * Update docs/commands/dev.md Co-authored-by: Sean C Davis * Update src/commands/dev/dev.ts Co-authored-by: Sean C Davis * Update src/commands/serve/index.ts Co-authored-by: Sean C Davis * Update docs/commands/serve.md Co-authored-by: Sean C Davis * feat: make flag internal * fix: format * fix: update docs --------- Co-authored-by: Sean C Davis --- src/commands/dev/dev.ts | 7 +++ src/commands/serve/index.ts | 6 ++ src/commands/serve/serve.ts | 1 + src/lib/edge-functions/proxy.ts | 10 +-- src/lib/spinner.ts | 22 ++----- src/utils/framework-server.ts | 2 - src/utils/proxy-server.ts | 3 + src/utils/proxy.ts | 63 ++++++++++++------- .../__snapshots__/edge-functions.test.ts.snap | 3 - .../commands/dev/edge-functions.test.ts | 35 ++++++++++- tests/integration/utils/fixture.ts | 9 ++- 11 files changed, 103 insertions(+), 58 deletions(-) delete mode 100644 tests/integration/commands/dev/__snapshots__/edge-functions.test.ts.snap diff --git a/src/commands/dev/dev.ts b/src/commands/dev/dev.ts index 5dc38e256d0..3c968a25e9b 100644 --- a/src/commands/dev/dev.ts +++ b/src/commands/dev/dev.ts @@ -220,6 +220,7 @@ export const dev = async (options: OptionValues, command: BaseCommand) => { config: mutatedConfig, configPath: configPathOverride, debug: options.debug, + disableEdgeFunctions: options.internalDisableEdgeFunctions, projectDir: command.workingDir, env, getUpdatedConfig, @@ -271,6 +272,12 @@ export const createDevCommand = (program: BaseCommand) => { .option('-d ,--dir ', 'dir with static files') .option('-f ,--functions ', 'specify a functions folder to serve') .option('-o ,--offline', 'disables any features that require network access') + .addOption( + new Option( + '--internal-disable-edge-functions', + "disables edge functions. use this if your environment doesn't support Deno. This option is internal and should not be used by end users.", + ).hideHelp(true), + ) .option( '-l, --live [subdomain]', 'start a public live session; optionally, supply a subdomain to generate a custom URL', diff --git a/src/commands/serve/index.ts b/src/commands/serve/index.ts index 8d6f88c1948..9beb1ece0dd 100644 --- a/src/commands/serve/index.ts +++ b/src/commands/serve/index.ts @@ -19,6 +19,12 @@ export const createServeCommand = (program: BaseCommand) => .option('-d ,--dir ', 'dir with static files') .option('-f ,--functions ', 'specify a functions folder to serve') .option('-o ,--offline', 'disables any features that require network access') + .addOption( + new Option( + '--internal-disable-edge-functions', + "disables edge functions. use this if your environment doesn't support Deno. This option is internal and should not be used by end users.", + ).hideHelp(true), + ) .addOption( new Option('--functionsPort ', 'Old, prefer --functions-port. Port of functions server') .argParser((value) => Number.parseInt(value)) diff --git a/src/commands/serve/serve.ts b/src/commands/serve/serve.ts index 757dfcd28c6..e09f8c5dcf4 100644 --- a/src/commands/serve/serve.ts +++ b/src/commands/serve/serve.ts @@ -147,6 +147,7 @@ export const serve = async (options: OptionValues, command: BaseCommand) => { config, configPath: configPathOverride, debug: options.debug, + disableEdgeFunctions: options.internalDisableEdgeFunctions, env, functionsRegistry, geolocationMode: options.geo, diff --git a/src/lib/edge-functions/proxy.ts b/src/lib/edge-functions/proxy.ts index e8401a274d2..850da6ef87a 100644 --- a/src/lib/edge-functions/proxy.ts +++ b/src/lib/edge-functions/proxy.ts @@ -25,15 +25,9 @@ const headersSymbol = Symbol('Edge Functions Headers') const LOCAL_HOST = '127.0.0.1' const getDownloadUpdateFunctions = () => { - // @ts-expect-error TS(7034) FIXME: Variable 'spinner' implicitly has type 'any' in so... Remove this comment to see the full error message - let spinner + let spinner: ReturnType - /** - * @param {Error=} error_ - */ - // @ts-expect-error TS(7006) FIXME: Parameter 'error_' implicitly has an 'any' type. - const onAfterDownload = (error_) => { - // @ts-expect-error TS(2345) FIXME: Argument of type '{ error: boolean; spinner: any; ... Remove this comment to see the full error message + const onAfterDownload = (error_: unknown) => { stopSpinner({ error: Boolean(error_), spinner }) } diff --git a/src/lib/spinner.ts b/src/lib/spinner.ts index fb02a2665f8..e7ee72d29aa 100644 --- a/src/lib/spinner.ts +++ b/src/lib/spinner.ts @@ -1,28 +1,18 @@ import logSymbols from 'log-symbols' -import ora from 'ora' +import ora, { Ora } from 'ora' /** * Creates a spinner with the following text - * @param {object} config - * @param {string} config.text - * @returns {ora.Ora} */ -// @ts-expect-error TS(7031) FIXME: Binding element 'text' implicitly has an 'any' typ... Remove this comment to see the full error message -export const startSpinner = ({ text }) => +export const startSpinner = ({ text }: { text: string }) => ora({ text, }).start() /** * Stops the spinner with the following text - * @param {object} config - * @param {ora.Ora} config.spinner - * @param {boolean} [config.error] - * @param {string} [config.text] - * @returns {void} */ -// @ts-expect-error TS(7031) FIXME: Binding element 'error' implicitly has an 'any' ty... Remove this comment to see the full error message -export const stopSpinner = ({ error, spinner, text }) => { +export const stopSpinner = ({ error, spinner, text }: { error: boolean; spinner: Ora; text?: string }) => { if (!spinner) { return } @@ -36,12 +26,8 @@ export const stopSpinner = ({ error, spinner, text }) => { /** * Clears the spinner - * @param {object} config - * @param {ora.Ora} config.spinner - * @returns {void} */ -// @ts-expect-error TS(7031) FIXME: Binding element 'spinner' implicitly has an 'any' ... Remove this comment to see the full error message -export const clearSpinner = ({ spinner }) => { +export const clearSpinner = ({ spinner }: { spinner: Ora }) => { if (spinner) { spinner.stop() } diff --git a/src/utils/framework-server.ts b/src/utils/framework-server.ts index 706a3907950..1b7e97b77bd 100644 --- a/src/utils/framework-server.ts +++ b/src/utils/framework-server.ts @@ -65,10 +65,8 @@ export const startFrameworkServer = async function ({ throw new Error(`Timed out waiting for port '${settings.frameworkPort}' to be open`) } } - // @ts-expect-error TS(2345) FIXME: Argument of type '{ error: boolean; spinner: Ora; ... Remove this comment to see the full error message stopSpinner({ error: false, spinner }) } catch (error_) { - // @ts-expect-error TS(2345) FIXME: Argument of type '{ error: boolean; spinner: Ora; ... Remove this comment to see the full error message stopSpinner({ error: true, spinner }) log(NETLIFYDEVERR, `Netlify Dev could not start or connect to localhost:${settings.frameworkPort}.`) log(NETLIFYDEVERR, `Please make sure your framework server is running on port ${settings.frameworkPort}`) diff --git a/src/utils/proxy-server.ts b/src/utils/proxy-server.ts index c450b2033a5..91e7476dec4 100644 --- a/src/utils/proxy-server.ts +++ b/src/utils/proxy-server.ts @@ -47,6 +47,7 @@ export const startProxyServer = async ({ config, configPath, debug, + disableEdgeFunctions, env, functionsRegistry, geoCountry, @@ -69,6 +70,7 @@ export const startProxyServer = async ({ // An override for the Netlify config path configPath?: string debug: boolean + disableEdgeFunctions: boolean env: NetlifyOptions['cachedConfig']['env'] inspectSettings: InspectSettings getUpdatedConfig: () => Promise @@ -90,6 +92,7 @@ export const startProxyServer = async ({ config, configPath: configPath || site.configPath, debug, + disableEdgeFunctions, env, functionsRegistry, geolocationMode, diff --git a/src/utils/proxy.ts b/src/utils/proxy.ts index c91e4705f7c..48694f62f00 100644 --- a/src/utils/proxy.ts +++ b/src/utils/proxy.ts @@ -680,7 +680,10 @@ const onRequest = async ( rewriter, settings, siteInfo, - }: { rewriter: Rewriter; settings: ServerSettings } & Record, + }: { rewriter: Rewriter; settings: ServerSettings; edgeFunctionsProxy?: EdgeFunctionsProxy } & Record< + string, + $TSFixMe + >, req: Request, res: ServerResponse, ) => { @@ -691,7 +694,7 @@ const onRequest = async ( return imageProxy(req, res) } - const edgeFunctionsProxyURL = await edgeFunctionsProxy(req, res) + const edgeFunctionsProxyURL = await edgeFunctionsProxy?.(req as any) if (edgeFunctionsProxyURL !== undefined) { return proxy.web(req, res, { target: edgeFunctionsProxyURL }) @@ -776,6 +779,8 @@ export const getProxyUrl = function (settings: Pick> + export const startProxy = async function ({ accountId, addonsUrls, @@ -784,6 +789,7 @@ export const startProxy = async function ({ config, configPath, debug, + disableEdgeFunctions, env, functionsRegistry, geoCountry, @@ -796,30 +802,39 @@ export const startProxy = async function ({ settings, siteInfo, state, -}: { command: BaseCommand; settings: ServerSettings } & Record) { +}: { command: BaseCommand; settings: ServerSettings; disableEdgeFunctions: boolean } & Record) { const secondaryServerPort = settings.https ? await getAvailablePort() : null const functionsServer = settings.functionsPort ? `http://127.0.0.1:${settings.functionsPort}` : null - const edgeFunctionsProxy = await initializeEdgeFunctionsProxy({ - command, - blobsContext, - config, - configPath, - debug, - env, - geolocationMode, - geoCountry, - getUpdatedConfig, - inspectSettings, - mainPort: settings.port, - offline, - passthroughPort: secondaryServerPort || settings.port, - settings, - projectDir, - repositoryRoot, - siteInfo, - accountId, - state, - }) + + let edgeFunctionsProxy: EdgeFunctionsProxy | undefined + if (disableEdgeFunctions) { + log( + NETLIFYDEVWARN, + 'Edge functions are disabled. Run without the --internal-disable-edge-functions flag to enable them.', + ) + } else { + edgeFunctionsProxy = await initializeEdgeFunctionsProxy({ + command, + blobsContext, + config, + configPath, + debug, + env, + geolocationMode, + geoCountry, + getUpdatedConfig, + inspectSettings, + mainPort: settings.port, + offline, + passthroughPort: secondaryServerPort || settings.port, + settings, + projectDir, + repositoryRoot, + siteInfo, + accountId, + state, + }) + } const imageProxy = await initializeImageProxy({ config, diff --git a/tests/integration/commands/dev/__snapshots__/edge-functions.test.ts.snap b/tests/integration/commands/dev/__snapshots__/edge-functions.test.ts.snap deleted file mode 100644 index 59223bc762d..00000000000 --- a/tests/integration/commands/dev/__snapshots__/edge-functions.test.ts.snap +++ /dev/null @@ -1,3 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[`edge functions > fixture: dev-server-with-edge-functions > should run edge functions in correct order 1`] = `"integration-manifestB|integration-manifestC|integration-manifestA|integration-iscA|integration-iscB|user-tomlB|user-tomlC|user-tomlA|user-iscA|user-iscB|origin"`; diff --git a/tests/integration/commands/dev/edge-functions.test.ts b/tests/integration/commands/dev/edge-functions.test.ts index 7e0268278d5..00b65da29d8 100644 --- a/tests/integration/commands/dev/edge-functions.test.ts +++ b/tests/integration/commands/dev/edge-functions.test.ts @@ -56,7 +56,19 @@ describe.skipIf(isWindows)('edge functions', () => { const body = await response.text() expect(response.status).toBe(200) - expect(body).toMatchSnapshot() + expect(body.split('|')).toEqual([ + 'integration-manifestB', + 'integration-manifestC', + 'integration-manifestA', + 'integration-iscA', + 'integration-iscB', + 'user-tomlB', + 'user-tomlC', + 'user-tomlA', + 'user-iscA', + 'user-iscB', + 'origin', + ]) }) test('should provide context properties', async ({ devServer }) => { @@ -147,6 +159,27 @@ describe.skipIf(isWindows)('edge functions', () => { }, ) + setupFixtureTests( + 'dev-server-with-edge-functions', + { + devServer: { args: ['--internal-disable-edge-functions'] }, + mockApi: { routes }, + setupAfterDev: recreateEdgeFunctions, + }, + () => { + test('skips edge functions when --internal-disable-edge-functions is passed', async ({ + devServer, + }) => { + const response = await fetch(`http://localhost:${devServer.port}/ordertest`) + const body = await response.text() + + expect(response.status).toBe(200) + expect(body).toEqual('origin\n') + expect(devServer?.output).toContain('Edge functions are disabled') + }) + }, + ) + setupFixtureTests('dev-server-with-edge-functions', { devServer: true, mockApi: { routes } }, () => { test('should not remove other edge functions on change', async ({ devServer, fixture }) => { // we need to wait till file watchers are loaded diff --git a/tests/integration/utils/fixture.ts b/tests/integration/utils/fixture.ts index a8480004ac9..23e84c26f30 100644 --- a/tests/integration/utils/fixture.ts +++ b/tests/integration/utils/fixture.ts @@ -27,7 +27,7 @@ export interface FixtureTestContext { type LifecycleHook = (context: FixtureTestContext) => Promise | void export interface FixtureOptions { - devServer?: boolean | { serve?: boolean } + devServer?: boolean | { serve?: boolean; args?: string[] } mockApi?: MockApiOptions /** * Executed after fixture setup, but before tests run @@ -151,11 +151,16 @@ export async function setupFixtureTests( await options.setup?.({ fixture, mockApi }) if (options.devServer) { + const args = ['--country', 'DE'] + if (typeof options.devServer === 'object' && options.devServer.args) { + args.push(...options.devServer.args) + } + devServer = await startDevServer({ serve: typeof options.devServer === 'object' && options.devServer.serve, cwd: fixture.directory, offline: !mockApi, - args: ['--country', 'DE'], + args, env: { NETLIFY_API_URL: mockApi?.apiUrl, NETLIFY_SITE_ID: 'foo',