Skip to content

Commit

Permalink
feat: replace glob-to-regexp with URLPattern (#392)
Browse files Browse the repository at this point in the history
* feat: replace `glob-to-regexp` with `URLPattern`

* chore: fix tests

* fix: pin version of `urlpattern-polyfill`

* chore: put behind featureflag

* fix: last missing test

* fix: types

---------

Co-authored-by: Simon Knott <info@simonknott.de>
  • Loading branch information
eduardoboucas and Skn0tt committed Jul 26, 2023
1 parent d9ae730 commit ca6962d
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 38 deletions.
3 changes: 2 additions & 1 deletion node/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ 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 All @@ -198,7 +199,7 @@ test('Loads function paths from the in-source `config` function', async () => {
expect(routes[2]).toEqual({ function: 'framework-func1', pattern: '^/framework-func1/?$', excluded_patterns: [] })
expect(routes[3]).toEqual({ function: 'user-func1', pattern: '^/user-func1/?$', excluded_patterns: [] })
expect(routes[4]).toEqual({ function: 'user-func3', pattern: '^/user-func3/?$', excluded_patterns: [] })
expect(routes[5]).toEqual({ function: 'user-func5', pattern: '^/user-func5/.*/?$', excluded_patterns: [] })
expect(routes[5]).toEqual({ function: 'user-func5', pattern: '^/user-func5(?:/(.*))/?$', excluded_patterns: [] })

expect(postCacheRoutes.length).toBe(1)
expect(postCacheRoutes[0]).toEqual({ function: 'user-func4', pattern: '^/user-func4/?$', excluded_patterns: [] })
Expand Down
1 change: 1 addition & 0 deletions node/feature_flags.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const defaultFlags = {
edge_functions_fail_unsupported_regex: false,
edge_functions_path_urlpattern: false,
}

type FeatureFlag = keyof typeof defaultFlags
Expand Down
82 changes: 60 additions & 22 deletions node/manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,15 @@ test('Generates a manifest with display names', () => {
name: 'Display Name',
},
}
const manifest = generateManifest({ bundles: [], declarations, functions, internalFunctionConfig })
const manifest = generateManifest({
bundles: [],
declarations,
functions,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})

const expectedRoutes = [{ function: 'func-1', pattern: '^/f1/.*/?$', excluded_patterns: [] }]
const expectedRoutes = [{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [] }]
expect(manifest.function_config).toEqual({
'func-1': { name: 'Display Name' },
})
Expand All @@ -63,9 +69,15 @@ test('Generates a manifest with a generator field', () => {
generator: '@netlify/fake-plugin@1.0.0',
},
}
const manifest = generateManifest({ bundles: [], declarations, functions, internalFunctionConfig })
const manifest = generateManifest({
bundles: [],
declarations,
functions,
internalFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})

const expectedRoutes = [{ function: 'func-1', pattern: '^/f1/.*/?$', excluded_patterns: [] }]
const expectedRoutes = [{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [] }]
const expectedFunctionConfig = { 'func-1': { generator: '@netlify/fake-plugin@1.0.0' } }
expect(manifest.routes).toEqual(expectedRoutes)
expect(manifest.function_config).toEqual(expectedFunctionConfig)
Expand All @@ -79,14 +91,23 @@ test('Generates a manifest with excluded paths and patterns', () => {
]
const declarations: Declaration[] = [
{ function: 'func-1', path: '/f1/*', excludedPath: '/f1/exclude' },
{ function: 'func-2', pattern: '^/f2/.*/?$', excludedPattern: ['^/f2/exclude$', '^/f2/exclude-as-well$'] },
{ function: 'func-2', pattern: '^/f2(?:/(.*))/?$', excludedPattern: ['^/f2/exclude$', '^/f2/exclude-as-well$'] },
{ function: 'func-3', path: '/*', excludedPath: '/**/*.html' },
]
const manifest = generateManifest({ bundles: [], declarations, functions })
const manifest = generateManifest({
bundles: [],
declarations,
functions,
featureFlags: { edge_functions_path_urlpattern: true },
})
const expectedRoutes = [
{ function: 'func-1', pattern: '^/f1/.*/?$', excluded_patterns: ['^/f1/exclude/?$'] },
{ function: 'func-2', pattern: '^/f2/.*/?$', excluded_patterns: ['^/f2/exclude$', '^/f2/exclude-as-well$'] },
{ function: 'func-3', pattern: '^/.*/?$', excluded_patterns: ['^/.*/.*\\.html/?$'] },
{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: ['^/f1/exclude/?$'] },
{ function: 'func-2', pattern: '^/f2(?:/(.*))/?$', excluded_patterns: ['^/f2/exclude$', '^/f2/exclude-as-well$'] },
{
function: 'func-3',
pattern: '^(?:/(.*))/?$',
excluded_patterns: ['^(?:/((?:.*)(?:/(?:.*))*))?(?:/(.*))\\.html/?$'],
},
]

expect(manifest.routes).toEqual(expectedRoutes)
Expand All @@ -105,9 +126,14 @@ test('TOML-defined paths can be combined with ISC-defined excluded paths', () =>
const userFunctionConfig: Record<string, FunctionConfig> = {
'func-1': { excludedPath: '/f1/exclude' },
}
const manifest = generateManifest({ bundles: [], declarations, functions, userFunctionConfig })

const expectedRoutes = [{ function: 'func-1', pattern: '^/f1/.*/?$', excluded_patterns: [] }]
const manifest = generateManifest({
bundles: [],
declarations,
functions,
userFunctionConfig,
featureFlags: { edge_functions_path_urlpattern: true },
})
const expectedRoutes = [{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [] }]

expect(manifest.routes).toEqual(expectedRoutes)
expect(manifest.function_config).toEqual({
Expand All @@ -123,7 +149,7 @@ test('Filters out internal in-source configurations in user created functions',
]
const declarations: Declaration[] = [
{ function: 'func-1', path: '/f1/*' },
{ function: 'func-2', pattern: '^/f2/.*/?$' },
{ function: 'func-2', pattern: '^/f2(?:/(.*))/?$' },
]
const userFunctionConfig: Record<string, FunctionConfig> = {
'func-1': {
Expand Down Expand Up @@ -185,22 +211,23 @@ 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([
{
function: 'customisation',
pattern: '^/showcases/.*/?$',
pattern: '^/showcases(?:/(.*))/?$',
excluded_patterns: [],
},
{
function: 'customisation',
pattern: '^/checkout/.*/?$',
excluded_patterns: ['^/.*/terms-and-conditions/?$'],
pattern: '^/checkout(?:/(.*))/?$',
excluded_patterns: ['^(?:/(.*))/terms-and-conditions/?$'],
},
])
expect(manifest.function_config).toEqual({
customisation: {
excluded_patterns: ['^/.*\\.css/?$', '^/.*\\.jpg/?$'],
excluded_patterns: ['^(?:/(.*))\\.css/?$', '^(?:/(.*))\\.jpg/?$'],
},
})

Expand All @@ -220,7 +247,7 @@ test('Includes failure modes in manifest', () => {
]
const declarations: Declaration[] = [
{ function: 'func-1', path: '/f1/*' },
{ function: 'func-2', pattern: '^/f2/.*/?$' },
{ function: 'func-2', pattern: '^/f2(?:/(.*))/?$' },
]
const userFunctionConfig: Record<string, FunctionConfig> = {
'func-1': {
Expand Down Expand Up @@ -330,17 +357,28 @@ test('Generates a manifest with layers', () => {
{ function: 'func-2', path: '/f2/*' },
]
const expectedRoutes = [
{ function: 'func-1', pattern: '^/f1/.*/?$', excluded_patterns: [] },
{ function: 'func-2', pattern: '^/f2/.*/?$', excluded_patterns: [] },
{ function: 'func-1', pattern: '^/f1(?:/(.*))/?$', excluded_patterns: [] },
{ function: 'func-2', pattern: '^/f2(?:/(.*))/?$', excluded_patterns: [] },
]
const layers = [
{
name: 'onion',
flag: 'edge_functions_onion_layer',
},
]
const manifest1 = generateManifest({ bundles: [], declarations, functions })
const manifest2 = generateManifest({ bundles: [], declarations, functions, layers })
const manifest1 = generateManifest({
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)
expect(manifest1.layers).toEqual([])
Expand Down
44 changes: 29 additions & 15 deletions node/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { FeatureFlags } from './feature_flags.js'
import { Layer } from './layer.js'
import { getPackageVersion } from './package_json.js'
import { nonNullable } from './utils/non_nullable.js'
import { ExtendedURLPattern } from './utils/urlpattern.js'

interface Route {
function: string
Expand Down Expand Up @@ -77,10 +78,11 @@ 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(pathToRegularExpression).map(serializePattern)
const excludedPatterns = paths.map((path) => pathToRegularExpression(path, featureFlags)).map(serializePattern)

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

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

manifestFunctionConfig[name] = { ...manifestFunctionConfig[name], on_error: onError, ...rest }
}
Expand All @@ -129,11 +131,8 @@ const generateManifest = ({
return
}

const pattern = getRegularExpression(declaration, featureFlags?.edge_functions_fail_unsupported_regex)
const excludedPattern = getExcludedRegularExpressions(
declaration,
featureFlags?.edge_functions_fail_unsupported_regex,
)
const pattern = getRegularExpression(declaration, featureFlags)
const excludedPattern = getExcludedRegularExpressions(declaration, featureFlags)

const route: Route = {
function: func.name,
Expand Down Expand Up @@ -164,7 +163,22 @@ const generateManifest = ({
return manifest
}

const pathToRegularExpression = (path: string) => {
const pathToRegularExpression = (path: string, featureFlags?: FeatureFlags) => {
if (featureFlags?.edge_functions_path_urlpattern) {
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)

// 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
}

// 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' })
Expand All @@ -177,13 +191,13 @@ const pathToRegularExpression = (path: string) => {
return normalizedSource
}

const getRegularExpression = (declaration: Declaration, failUnsupportedRegex = false): string => {
const getRegularExpression = (declaration: Declaration, featureFlags?: FeatureFlags): string => {
if ('pattern' in declaration) {
try {
return parsePattern(declaration.pattern)
} catch (error: unknown) {
// eslint-disable-next-line max-depth
if (failUnsupportedRegex) {
if (featureFlags?.edge_functions_fail_unsupported_regex) {
throw new Error(
`Could not parse path declaration of function '${declaration.function}': ${(error as Error).message}`,
)
Expand All @@ -199,10 +213,10 @@ const getRegularExpression = (declaration: Declaration, failUnsupportedRegex = f
}
}

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

const getExcludedRegularExpressions = (declaration: Declaration, failUnsupportedRegex = false): string[] => {
const getExcludedRegularExpressions = (declaration: Declaration, featureFlags?: FeatureFlags): string[] => {
if ('excludedPattern' in declaration && declaration.excludedPattern) {
const excludedPatterns: string[] = Array.isArray(declaration.excludedPattern)
? declaration.excludedPattern
Expand All @@ -211,7 +225,7 @@ const getExcludedRegularExpressions = (declaration: Declaration, failUnsupported
try {
return parsePattern(excludedPattern)
} catch (error: unknown) {
if (failUnsupportedRegex) {
if (featureFlags?.edge_functions_fail_unsupported_regex) {
throw new Error(
`Could not parse path declaration of function '${declaration.function}': ${(error as Error).message}`,
)
Expand All @@ -230,7 +244,7 @@ const getExcludedRegularExpressions = (declaration: Declaration, failUnsupported

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

return []
Expand Down
7 changes: 7 additions & 0 deletions node/utils/urlpattern.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { URLPattern } from 'urlpattern-polyfill'

export class ExtendedURLPattern extends URLPattern {
// @ts-expect-error Internal property that the underlying class is using but
// not exposing.
regexp: Record<string, RegExp>
}
11 changes: 11 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"regexp-tree": "^0.1.24",
"semver": "^7.3.8",
"tmp-promise": "^3.0.3",
"urlpattern-polyfill": "8.0.2",
"uuid": "^9.0.0"
}
}

0 comments on commit ca6962d

Please sign in to comment.