Skip to content
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.

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

feat: remove URLPattern feature flag #460

Merged
merged 1 commit into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion node/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ test('Loads function paths from the in-source `config` function', async () => {
const result = await bundle([internalDirectory, userDirectory], distPath, declarations, {
basePath,
configPath: join(internalDirectory, 'config.json'),
featureFlags: { edge_functions_path_urlpattern: true },
})
const generatedFiles = await fs.readdir(distPath)

Expand Down
1 change: 0 additions & 1 deletion node/feature_flags.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const defaultFlags = {
edge_functions_fail_unsupported_regex: false,
edge_functions_path_urlpattern: false,
}

type FeatureFlag = keyof typeof defaultFlags
Expand Down
9 changes: 0 additions & 9 deletions node/manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ test('Generates a manifest with display names', () => {
declarations,
functions,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})

const expectedRoutes = [{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [], path: '/f1/*' }]
Expand All @@ -75,7 +74,6 @@ test('Generates a manifest with a generator field', () => {
declarations,
functions,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})

const expectedRoutes = [{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [], path: '/f1/*' }]
Expand All @@ -99,7 +97,6 @@ test('Generates a manifest with excluded paths and patterns', () => {
bundles: [],
declarations,
functions,
featureFlags: { edge_functions_path_urlpattern: true },
})
const expectedRoutes = [
{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: ['^/f1/exclude/?$'], path: '/f1/*' },
Expand Down Expand Up @@ -137,7 +134,6 @@ test('TOML-defined paths can be combined with ISC-defined excluded paths', () =>
declarations,
functions,
userFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})
const expectedRoutes = [{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [], path: '/f1/*' }]

Expand Down Expand Up @@ -217,7 +213,6 @@ test('excludedPath from ISC goes into function_config, TOML goes into routes', (
functions,
userFunctionConfig,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})
expect(manifest.routes).toEqual([
{
Expand Down Expand Up @@ -259,7 +254,6 @@ test('URLPattern named groups are supported', () => {
functions,
userFunctionConfig,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})
expect(manifest.routes).toEqual([
{
Expand Down Expand Up @@ -288,7 +282,6 @@ test('Invalid Path patterns throw bundling errors', () => {
functions,
userFunctionConfig,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
}),
).toThrowError(BundleError)
})
Expand Down Expand Up @@ -425,14 +418,12 @@ test('Generates a manifest with layers', () => {
bundles: [],
declarations,
functions,
featureFlags: { edge_functions_path_urlpattern: true },
})
const manifest2 = generateManifest({
bundles: [],
declarations,
functions,
layers,
featureFlags: { edge_functions_path_urlpattern: true },
})

expect(manifest1.routes).toEqual(expectedRoutes)
Expand Down
52 changes: 18 additions & 34 deletions node/manifest.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { promises as fs } from 'fs'
import { join } from 'path'

import globToRegExp from 'glob-to-regexp'

import type { Bundle } from './bundle.js'
import { wrapBundleError } from './bundle_error.js'
import { Cache, FunctionConfig, Path } from './config.js'
Expand Down Expand Up @@ -80,11 +78,10 @@ const addExcludedPatterns = (
name: string,
manifestFunctionConfig: Record<string, EdgeFunctionConfig>,
excludedPath?: Path | Path[],
featureFlags?: FeatureFlags,
) => {
if (excludedPath) {
const paths = Array.isArray(excludedPath) ? excludedPath : [excludedPath]
const excludedPatterns = paths.map((path) => pathToRegularExpression(path, featureFlags)).map(serializePattern)
const excludedPatterns = paths.map((path) => pathToRegularExpression(path)).map(serializePattern)

manifestFunctionConfig[name].excluded_patterns.push(...excludedPatterns)
}
Expand All @@ -111,7 +108,7 @@ const generateManifest = ({
if (manifestFunctionConfig[name] === undefined) {
continue
}
addExcludedPatterns(name, manifestFunctionConfig, excludedPath, featureFlags)
addExcludedPatterns(name, manifestFunctionConfig, excludedPath)

manifestFunctionConfig[name] = { ...manifestFunctionConfig[name], on_error: onError }
}
Expand All @@ -121,7 +118,7 @@ const generateManifest = ({
if (manifestFunctionConfig[name] === undefined) {
continue
}
addExcludedPatterns(name, manifestFunctionConfig, excludedPath, featureFlags)
addExcludedPatterns(name, manifestFunctionConfig, excludedPath)

manifestFunctionConfig[name] = { ...manifestFunctionConfig[name], on_error: onError, ...rest }
}
Expand Down Expand Up @@ -169,36 +166,23 @@ const generateManifest = ({
return manifest
}

const pathToRegularExpression = (path: string, featureFlags?: FeatureFlags) => {
if (featureFlags?.edge_functions_path_urlpattern) {
try {
const pattern = new ExtendedURLPattern({ pathname: path })
const pathToRegularExpression = (path: string) => {
try {
const pattern = new ExtendedURLPattern({ pathname: path })

// Removing the `^` and `$` delimiters because we'll need to modify what's
// between them.
const source = pattern.regexp.pathname.source.slice(1, -1)
// Removing the `^` and `$` delimiters because we'll need to modify what's
// between them.
const source = pattern.regexp.pathname.source.slice(1, -1)

// Wrapping the expression source with `^` and `$`. Also, adding an optional
// trailing slash, so that a declaration of `path: "/foo"` matches requests
// for both `/foo` and `/foo/`.
const normalizedSource = `^${source}\\/?$`
// Wrapping the expression source with `^` and `$`. Also, adding an optional
// trailing slash, so that a declaration of `path: "/foo"` matches requests
// for both `/foo` and `/foo/`.
const normalizedSource = `^${source}\\/?$`

return normalizedSource
} catch (error) {
throw wrapBundleError(error)
}
return normalizedSource
} catch (error) {
throw wrapBundleError(error)
}

// We use the global flag so that `globToRegExp` will not wrap the expression
// with `^` and `$`. We'll do that ourselves.
const regularExpression = globToRegExp(path, { flags: 'g' })

// Wrapping the expression source with `^` and `$`. Also, adding an optional
// trailing slash, so that a declaration of `path: "/foo"` matches requests
// for both `/foo` and `/foo/`.
const normalizedSource = `^${regularExpression.source}\\/?$`

return normalizedSource
}

const getRegularExpression = (declaration: Declaration, featureFlags?: FeatureFlags): string => {
Expand All @@ -223,7 +207,7 @@ const getRegularExpression = (declaration: Declaration, featureFlags?: FeatureFl
}
}

return pathToRegularExpression(declaration.path, featureFlags)
return pathToRegularExpression(declaration.path)
}

const getExcludedRegularExpressions = (declaration: Declaration, featureFlags?: FeatureFlags): string[] => {
Expand Down Expand Up @@ -254,7 +238,7 @@ const getExcludedRegularExpressions = (declaration: Declaration, featureFlags?:

if ('path' in declaration && declaration.excludedPath) {
const paths = Array.isArray(declaration.excludedPath) ? declaration.excludedPath : [declaration.excludedPath]
return paths.map((path) => pathToRegularExpression(path, featureFlags))
return paths.map((path) => pathToRegularExpression(path))
}

return []
Expand Down
11 changes: 0 additions & 11 deletions package-lock.json

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

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
"execa": "^6.0.0",
"find-up": "^6.3.0",
"get-port": "^6.1.2",
"glob-to-regexp": "^0.4.1",
"is-path-inside": "^4.0.0",
"jsonc-parser": "^3.2.0",
"node-fetch": "^3.1.1",
Expand Down