feat: remove feature flag for PCRE engine (#580)

* feat: remove flag for PCRE engine

* fix: remove flag entirely

* refactor: rename method

node/declaration.test.ts

@@ -1,7 +1,7 @@
 import { test, expect } from 'vitest'
 
 import { FunctionConfig } from './config.js'
-import { Declaration, mergeDeclarations, parsePattern } from './declaration.js'
+import { Declaration, mergeDeclarations, normalizePattern } from './declaration.js'
 
 const deployConfigDeclarations: Declaration[] = []
 
@@ -164,22 +164,19 @@ test('Does not escape front slashes in a regex pattern if they are already escap
   const regexPattern = '^(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/([^/.]{1,}))\\/shows(?:\\/(.*))(.json)?[\\/#\\?]?$'
   const expected = '^(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/([^/.]{1,}))\\/shows(?:\\/(.*))(.json)?[\\/#\\?]?$'
 
-  expect(parsePattern(regexPattern, false)).toEqual(expected)
-  expect(parsePattern(regexPattern, true)).toEqual(expected)
+  expect(normalizePattern(regexPattern)).toEqual(expected)
 })
 
 test('Escapes front slashes in a regex pattern', () => {
   const regexPattern = '^(?:/(_next/data/[^/]{1,}))?(?:/([^/.]{1,}))/shows(?:/(.*))(.json)?[/#\\?]?$'
   const expected = '^(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/([^/.]{1,}))\\/shows(?:\\/(.*))(.json)?[/#\\?]?$'
 
-  expect(parsePattern(regexPattern, false)).toEqual(expected)
-  expect(parsePattern(regexPattern, true)).toEqual(expected)
+  expect(normalizePattern(regexPattern)).toEqual(expected)
 })
 
 test('Ensures pattern match on the whole path', () => {
   const regexPattern = '/foo/.*/bar'
   const expected = '^\\/foo\\/.*\\/bar$'
 
-  expect(parsePattern(regexPattern, false)).toEqual(expected)
-  expect(parsePattern(regexPattern, true)).toEqual(expected)
+  expect(normalizePattern(regexPattern)).toEqual(expected)
 })

node/declaration.ts

@@ -1,5 +1,3 @@
-import regexpAST from 'regexp-tree'
-
 import { FunctionConfig, HTTPMethod, Path } from './config.js'
 import { FeatureFlags } from './feature_flags.js'
 
@@ -118,10 +116,9 @@ const createDeclarationsFromFunctionConfigs = (
 
 /**
  * Normalizes a regular expression, ensuring it has a leading `^` and trailing
- * `$` characters. It also converts the regular expression from PCRE to RE2 if
- * needed.
+ * `$` characters.
  */
-export const parsePattern = (pattern: string, pcreRegexpEngine: boolean) => {
+export const normalizePattern = (pattern: string) => {
   let enclosedPattern = pattern
 
   if (!pattern.startsWith('^')) {
@@ -133,38 +130,7 @@ export const parsePattern = (pattern: string, pcreRegexpEngine: boolean) => {
   }
 
   const regexp = new RegExp(enclosedPattern)
-  const regexpString = pcreRegexpEngine ? regexp.toString() : transformPCRERegexp(regexp)
 
   // Strip leading and forward slashes.
-  return regexpString.slice(1, -1)
-}
-
-/**
- * Transforms a PCRE regular expression into a RE2 expression, compatible
- * with the Go engine used in our edge nodes.
- */
-const transformPCRERegexp = (regexp: RegExp) => {
-  const newRegexp = regexpAST.transform(regexp, {
-    Assertion(path) {
-      // Lookaheads are not supported. If we find one, throw an error.
-      if (path.node.kind === 'Lookahead') {
-        throw new Error('Regular expressions with lookaheads are not supported')
-      }
-    },
-
-    Group(path) {
-      // Named captured groups in JavaScript use a different syntax than in Go.
-      // If we find one, convert it to an unnamed capture group, which is valid
-      // in both engines.
-      if ('name' in path.node && path.node.name !== undefined) {
-        path.replace({
-          ...path.node,
-          name: undefined,
-          nameRaw: undefined,
-        })
-      }
-    },
-  })
-
-  return newRegexp.toString()
+  return regexp.toString().slice(1, -1)
 }

node/feature_flags.ts

@@ -1,6 +1,4 @@
-const defaultFlags = {
-  edge_bundler_pcre_regexp: false,
-}
+const defaultFlags = {}
 
 type FeatureFlag = keyof typeof defaultFlags
 type FeatureFlags = Partial<Record<FeatureFlag, boolean>>

node/manifest.test.ts

@@ -433,28 +433,12 @@ test('Generates a manifest with layers', () => {
   expect(manifest2.layers).toEqual(layers)
 })
 
-test('Throws an error if the regular expression contains a negative lookahead and support for the PCRE engine is disabled', () => {
-  const functions = [{ name: 'func-1', path: '/path/to/func-1.ts' }]
-  const declarations = [{ function: 'func-1', pattern: '^\\/((?!api|_next\\/static|_next\\/image|favicon.ico).*)$' }]
-
-  expect(() =>
-    generateManifest({
-      bundles: [],
-      declarations,
-      functions,
-    }),
-  ).toThrowError(
-    /^Could not parse path declaration of function 'func-1': Regular expressions with lookaheads are not supported$/,
-  )
-})
-
-test('Accepts regular expressions with lookaheads if support for the PCRE engine is enabled', () => {
+test('Accepts regular expressions with lookaheads', () => {
   const functions = [{ name: 'func-1', path: '/path/to/func-1.ts' }]
   const declarations = [{ function: 'func-1', pattern: '^\\/((?!api|_next\\/static|_next\\/image|favicon.ico).*)$' }]
   const { manifest } = generateManifest({
     bundles: [],
     declarations,
-    featureFlags: { edge_bundler_pcre_regexp: true },
     functions,
   })
   const [route] = manifest.routes
@@ -464,12 +448,12 @@ test('Accepts regular expressions with lookaheads if support for the PCRE engine
   expect(regexp.test('/_next/static/foo')).toBeFalsy()
 })
 
-test('Converts named capture groups to unnamed capture groups in regular expressions', () => {
+test('Accepts regular expressions with named capture groups', () => {
   const functions = [{ name: 'func-1', path: '/path/to/func-1.ts' }]
   const declarations = [{ function: 'func-1', pattern: '^/(?<name>\\w+)$' }]
   const { manifest } = generateManifest({ bundles: [], declarations, functions })
 
-  expect(manifest.routes).toEqual([{ function: 'func-1', pattern: '^/(\\w+)$', excluded_patterns: [] }])
+  expect(manifest.routes).toEqual([{ function: 'func-1', pattern: '^/(?<name>\\w+)$', excluded_patterns: [] }])
 })
 
 test('Returns functions without a declaration and unrouted functions', () => {

node/manifest.ts

@@ -4,7 +4,7 @@ import { join } from 'path'
 import type { Bundle } from './bundle.js'
 import { wrapBundleError } from './bundle_error.js'
 import { Cache, FunctionConfig, Path } from './config.js'
-import { Declaration, parsePattern } from './declaration.js'
+import { Declaration, normalizePattern } from './declaration.js'
 import { EdgeFunction } from './edge_function.js'
 import { FeatureFlags } from './feature_flags.js'
 import { Layer } from './layer.js'
@@ -108,7 +108,6 @@ const normalizeMethods = (method: unknown, name: string): string[] | undefined =
 const generateManifest = ({
   bundles = [],
   declarations = [],
-  featureFlags,
   functions,
   userFunctionConfig = {},
   internalFunctionConfig = {},
@@ -154,7 +153,7 @@ const generateManifest = ({
       return
     }
 
-    const pattern = getRegularExpression(declaration, Boolean(featureFlags?.edge_bundler_pcre_regexp))
+    const pattern = getRegularExpression(declaration)
 
     // If there is no `pattern`, the declaration will never be triggered, so we
     // can discard it.
@@ -164,7 +163,7 @@ const generateManifest = ({
 
     routedFunctions.add(declaration.function)
 
-    const excludedPattern = getExcludedRegularExpressions(declaration, Boolean(featureFlags?.edge_bundler_pcre_regexp))
+    const excludedPattern = getExcludedRegularExpressions(declaration)
     const route: Route = {
       function: func.name,
       pattern: serializePattern(pattern),
@@ -226,10 +225,10 @@ const pathToRegularExpression = (path: string) => {
   }
 }
 
-const getRegularExpression = (declaration: Declaration, pcreRegexpEngine: boolean) => {
+const getRegularExpression = (declaration: Declaration) => {
   if ('pattern' in declaration) {
     try {
-      return parsePattern(declaration.pattern, pcreRegexpEngine)
+      return normalizePattern(declaration.pattern)
     } catch (error: unknown) {
       throw wrapBundleError(
         new Error(
@@ -242,15 +241,15 @@ const getRegularExpression = (declaration: Declaration, pcreRegexpEngine: boolea
   return pathToRegularExpression(declaration.path)
 }
 
-const getExcludedRegularExpressions = (declaration: Declaration, pcreRegexpEngine: boolean): string[] => {
+const getExcludedRegularExpressions = (declaration: Declaration): string[] => {
   if ('excludedPattern' in declaration && declaration.excludedPattern) {
     const excludedPatterns: string[] = Array.isArray(declaration.excludedPattern)
       ? declaration.excludedPattern
       : [declaration.excludedPattern]
 
     return excludedPatterns.map((excludedPattern) => {
       try {
-        return parsePattern(excludedPattern, pcreRegexpEngine)
+        return normalizePattern(excludedPattern)
       } catch (error: unknown) {
         throw wrapBundleError(
           new Error(

package.json

@@ -93,7 +93,6 @@
     "p-retry": "^5.1.1",
     "p-wait-for": "^4.1.0",
     "path-key": "^4.0.0",
-    "regexp-tree": "^0.1.24",
     "semver": "^7.3.8",
     "tmp-promise": "^3.0.3",
     "urlpattern-polyfill": "8.0.2",