feat: replace glob-to-regexp with URLPattern (#392)

* 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 <[email protected]>
netlify · Jul 26, 2023 · ca6962d · ca6962d
1 parent d9ae730
commit ca6962d
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 38 deletions.
diff --git a/node/config.test.ts b/node/config.test.ts
@@ -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)
 
@@ -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: [] })

diff --git a/node/feature_flags.ts b/node/feature_flags.ts
@@ -1,5 +1,6 @@
 const defaultFlags = {
   edge_functions_fail_unsupported_regex: false,
+  edge_functions_path_urlpattern: false,
 }
 
 type FeatureFlag = keyof typeof defaultFlags

diff --git a/node/manifest.test.ts b/node/manifest.test.ts
@@ -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' },
   })
@@ -63,9 +69,15 @@ test('Generates a manifest with a generator field', () => {
       generator: '@netlify/[email protected]',
     },
   }
-  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/[email protected]' } }
   expect(manifest.routes).toEqual(expectedRoutes)
   expect(manifest.function_config).toEqual(expectedFunctionConfig)
@@ -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)
@@ -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({
@@ -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': {
@@ -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/?$'],
     },
   })
 
@@ -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': {
@@ -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([])

diff --git a/node/manifest.ts b/node/manifest.ts
@@ -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
@@ -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)
   }
@@ -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 }
   }
@@ -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 }
   }
@@ -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,
@@ -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' })
@@ -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}`,
         )
@@ -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
@@ -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}`,
           )
@@ -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 []

diff --git a/node/utils/urlpattern.ts b/node/utils/urlpattern.ts
@@ -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>
+}
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -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"
   }
 }