Skip to content

Commit

Permalink
feat: add --internal-disable-edge-functions flag to skip Deno setup (
Browse files Browse the repository at this point in the history
…#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 <scdavis41@gmail.com>

* Update src/commands/dev/dev.ts

Co-authored-by: Sean C Davis <scdavis41@gmail.com>

* Update src/commands/serve/index.ts

Co-authored-by: Sean C Davis <scdavis41@gmail.com>

* Update docs/commands/serve.md

Co-authored-by: Sean C Davis <scdavis41@gmail.com>

* feat: make flag internal

* fix: format

* fix: update docs

---------

Co-authored-by: Sean C Davis <scdavis41@gmail.com>
  • Loading branch information
Skn0tt and seancdavis committed Apr 8, 2024
1 parent bc24f77 commit a6f0f45
Show file tree
Hide file tree
Showing 11 changed files with 103 additions and 58 deletions.
7 changes: 7 additions & 0 deletions src/commands/dev/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -271,6 +272,12 @@ export const createDevCommand = (program: BaseCommand) => {
.option('-d ,--dir <path>', 'dir with static files')
.option('-f ,--functions <folder>', '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',
Expand Down
6 changes: 6 additions & 0 deletions src/commands/serve/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ export const createServeCommand = (program: BaseCommand) =>
.option('-d ,--dir <path>', 'dir with static files')
.option('-f ,--functions <folder>', '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 <port>', 'Old, prefer --functions-port. Port of functions server')
.argParser((value) => Number.parseInt(value))
Expand Down
1 change: 1 addition & 0 deletions src/commands/serve/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 2 additions & 8 deletions src/lib/edge-functions/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof startSpinner>

/**
* @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 })
}

Expand Down
22 changes: 4 additions & 18 deletions src/lib/spinner.ts
Original file line number Diff line number Diff line change
@@ -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
}
Expand All @@ -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()
}
Expand Down
2 changes: 0 additions & 2 deletions src/utils/framework-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
Expand Down
3 changes: 3 additions & 0 deletions src/utils/proxy-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const startProxyServer = async ({
config,
configPath,
debug,
disableEdgeFunctions,
env,
functionsRegistry,
geoCountry,
Expand All @@ -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<object>
Expand All @@ -90,6 +92,7 @@ export const startProxyServer = async ({
config,
configPath: configPath || site.configPath,
debug,
disableEdgeFunctions,
env,
functionsRegistry,
geolocationMode,
Expand Down
63 changes: 39 additions & 24 deletions src/utils/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,10 @@ const onRequest = async (
rewriter,
settings,
siteInfo,
}: { rewriter: Rewriter; settings: ServerSettings } & Record<string, $TSFixMe>,
}: { rewriter: Rewriter; settings: ServerSettings; edgeFunctionsProxy?: EdgeFunctionsProxy } & Record<
string,
$TSFixMe
>,
req: Request,
res: ServerResponse,
) => {
Expand All @@ -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 })
Expand Down Expand Up @@ -776,6 +779,8 @@ export const getProxyUrl = function (settings: Pick<ServerSettings, 'https' | 'p
return `${scheme}://localhost:${settings.port}`
}

type EdgeFunctionsProxy = Awaited<ReturnType<typeof initializeEdgeFunctionsProxy>>

export const startProxy = async function ({
accountId,
addonsUrls,
Expand All @@ -784,6 +789,7 @@ export const startProxy = async function ({
config,
configPath,
debug,
disableEdgeFunctions,
env,
functionsRegistry,
geoCountry,
Expand All @@ -796,30 +802,39 @@ export const startProxy = async function ({
settings,
siteInfo,
state,
}: { command: BaseCommand; settings: ServerSettings } & Record<string, $TSFixMe>) {
}: { command: BaseCommand; settings: ServerSettings; disableEdgeFunctions: boolean } & Record<string, $TSFixMe>) {
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,
Expand Down

This file was deleted.

35 changes: 34 additions & 1 deletion tests/integration/commands/dev/edge-functions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<FixtureTestContext>('should provide context properties', async ({ devServer }) => {
Expand Down Expand Up @@ -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<FixtureTestContext>('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<FixtureTestContext>('should not remove other edge functions on change', async ({ devServer, fixture }) => {
// we need to wait till file watchers are loaded
Expand Down
9 changes: 7 additions & 2 deletions tests/integration/utils/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface FixtureTestContext {
type LifecycleHook = (context: FixtureTestContext) => Promise<void> | void

export interface FixtureOptions {
devServer?: boolean | { serve?: boolean }
devServer?: boolean | { serve?: boolean; args?: string[] }
mockApi?: MockApiOptions
/**
* Executed after fixture setup, but before tests run
Expand Down Expand Up @@ -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',
Expand Down

2 comments on commit a6f0f45

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Dependency count: 1,312
  • Package size: 294 MB
  • Number of ts-expect-error directives: 1,008

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Dependency count: 1,312
  • Package size: 294 MB
  • Number of ts-expect-error directives: 1,008

Please sign in to comment.