fix: remove npm_modules and fail_unsupported_regex flags (#514)

* fix: remove npm_modules and fail_unsupported_regex flags

* fix: make it a user error

node/bundler.test.ts

@@ -125,34 +125,7 @@ test('Adds a custom error property to user errors during bundling', async () =>
   }
 })
 
-test('Prints a nice error message when user tries importing an npm module and npm support is disabled', async () => {
-  expect.assertions(2)
-
-  const { basePath, cleanup, distPath } = await useFixture('imports_npm_module')
-  const sourceDirectory = join(basePath, 'functions')
-  const declarations: Declaration[] = [
-    {
-      function: 'func1',
-      path: '/func1',
-    },
-  ]
-
-  try {
-    await bundle([sourceDirectory], distPath, declarations, {
-      basePath,
-      importMapPaths: [join(basePath, 'import_map.json')],
-    })
-  } catch (error) {
-    expect(error).toBeInstanceOf(BundleError)
-    expect((error as BundleError).message).toEqual(
-      `It seems like you're trying to import an npm module. This is only supported via CDNs like esm.sh. Have you tried 'import mod from "https://esm.sh/parent-1"'?`,
-    )
-  } finally {
-    await cleanup()
-  }
-})
-
-test('Prints a nice error message when user tries importing an npm module and npm support is enabled', async () => {
+test('Prints a nice error message when user tries importing an npm module', async () => {
   expect.assertions(2)
 
   const { basePath, cleanup, distPath } = await useFixture('imports_npm_module_scheme')
@@ -167,7 +140,6 @@ test('Prints a nice error message when user tries importing an npm module and np
   try {
     await bundle([sourceDirectory], distPath, declarations, {
       basePath,
-      featureFlags: { edge_functions_npm_modules: true },
     })
   } catch (error) {
     expect(error).toBeInstanceOf(BundleError)
@@ -179,30 +151,6 @@ test('Prints a nice error message when user tries importing an npm module and np
   }
 })
 
-test('Prints a nice error message when user tries importing NPM module with npm: scheme', async () => {
-  expect.assertions(2)
-
-  const { basePath, cleanup, distPath } = await useFixture('imports_npm_module_scheme')
-  const sourceDirectory = join(basePath, 'functions')
-  const declarations: Declaration[] = [
-    {
-      function: 'func1',
-      path: '/func1',
-    },
-  ]
-
-  try {
-    await bundle([sourceDirectory], distPath, declarations, { basePath })
-  } catch (error) {
-    expect(error).toBeInstanceOf(BundleError)
-    expect((error as BundleError).message).toEqual(
-      `It seems like you're trying to import an npm module. This is only supported via CDNs like esm.sh. Have you tried 'import mod from "https://esm.sh/p-retry"'?`,
-    )
-  } finally {
-    await cleanup()
-  }
-})
-
 test('Does not add a custom error property to system errors during bundling', async () => {
   expect.assertions(1)
 
@@ -502,7 +450,6 @@ test('Loads npm modules from bare specifiers', async () => {
 
   await bundle([sourceDirectory], distPath, declarations, {
     basePath,
-    featureFlags: { edge_functions_npm_modules: true },
     importMapPaths: [join(basePath, 'import_map.json')],
     vendorDirectory: vendorDirectory.path,
     systemLogger,

node/bundler.ts

@@ -255,16 +255,11 @@ interface VendorNPMOptions {
 
 const safelyVendorNPMSpecifiers = async ({
   basePath,
-  featureFlags,
   functions,
   importMap,
   logger,
   vendorDirectory,
 }: VendorNPMOptions) => {
-  if (!featureFlags.edge_functions_npm_modules) {
-    return
-  }
-
   try {
     return await vendorNPMSpecifiers({
       basePath,

node/feature_flags.ts

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

node/formats/eszip.ts

@@ -33,7 +33,6 @@ const bundleESZIP = async ({
   deno,
   distDirectory,
   externals,
-  featureFlags,
   functions,
   importMap,
   vendorDirectory,
@@ -67,7 +66,7 @@ const bundleESZIP = async ({
   try {
     await deno.run(['run', ...flags, bundler, JSON.stringify(payload)], { pipeOutput: true })
   } catch (error: unknown) {
-    throw wrapBundleError(wrapNpmImportError(error, Boolean(featureFlags.edge_functions_npm_modules)), {
+    throw wrapBundleError(wrapNpmImportError(error), {
       format: 'eszip',
     })
   }

node/manifest.test.ts

@@ -1,6 +1,6 @@
 import { env } from 'process'
 
-import { test, expect, vi } from 'vitest'
+import { test, expect } from 'vitest'
 
 import { getRouteMatcher } from '../test/util.js'
 
@@ -433,38 +433,14 @@ test('Generates a manifest with layers', () => {
   expect(manifest2.layers).toEqual(layers)
 })
 
-test('Shows a warning if the regular expression contains a negative lookahead', () => {
-  const mockConsoleWarn = vi.fn()
-  const consoleWarn = console.warn
-
-  console.warn = mockConsoleWarn
-
-  const functions = [{ name: 'func-1', path: '/path/to/func-1.ts' }]
-  const declarations = [{ function: 'func-1', pattern: '^/\\w+(?=\\d)$' }]
-  const manifest = generateManifest({
-    bundles: [],
-    declarations,
-    functions,
-  })
-
-  console.warn = consoleWarn
-
-  expect(manifest.routes).toEqual([{ function: 'func-1', pattern: '^/\\w+(?=\\d)$', excluded_patterns: [] }])
-  expect(mockConsoleWarn).toHaveBeenCalledOnce()
-  expect(mockConsoleWarn).toHaveBeenCalledWith(
-    "Function 'func-1' uses an unsupported regular expression and will not be invoked: Regular expressions with lookaheads are not supported",
-  )
-})
-
-test('Throws an error if the regular expression contains a negative lookahead and the `edge_functions_fail_unsupported_regex` flag is set', () => {
+test('Throws an error if the regular expression contains a negative lookahead', () => {
   const functions = [{ name: 'func-1', path: '/path/to/func-1.ts' }]
   const declarations = [{ function: 'func-1', pattern: '^/\\w+(?=\\d)$' }]
 
   expect(() =>
     generateManifest({
       bundles: [],
       declarations,
-      featureFlags: { edge_functions_fail_unsupported_regex: true },
       functions,
     }),
   ).toThrowError(

node/manifest.ts

@@ -108,7 +108,6 @@ const normalizeMethods = (method: unknown, name: string): string[] | undefined =
 const generateManifest = ({
   bundles = [],
   declarations = [],
-  featureFlags,
   functions,
   userFunctionConfig = {},
   internalFunctionConfig = {},
@@ -148,8 +147,8 @@ const generateManifest = ({
       return
     }
 
-    const pattern = getRegularExpression(declaration, featureFlags)
-    const excludedPattern = getExcludedRegularExpressions(declaration, featureFlags)
+    const pattern = getRegularExpression(declaration)
+    const excludedPattern = getExcludedRegularExpressions(declaration)
 
     const route: Route = {
       function: func.name,
@@ -207,32 +206,23 @@ const pathToRegularExpression = (path: string) => {
   }
 }
 
-const getRegularExpression = (declaration: Declaration, featureFlags?: FeatureFlags): string => {
+const getRegularExpression = (declaration: Declaration): string => {
   if ('pattern' in declaration) {
     try {
       return parsePattern(declaration.pattern)
     } catch (error: unknown) {
-      // eslint-disable-next-line max-depth
-      if (featureFlags?.edge_functions_fail_unsupported_regex) {
-        throw new Error(
+      throw wrapBundleError(
+        new Error(
           `Could not parse path declaration of function '${declaration.function}': ${(error as Error).message}`,
-        )
-      }
-
-      console.warn(
-        `Function '${declaration.function}' uses an unsupported regular expression and will not be invoked: ${
-          (error as Error).message
-        }`,
+        ),
       )
-
-      return declaration.pattern
     }
   }
 
   return pathToRegularExpression(declaration.path)
 }
 
-const getExcludedRegularExpressions = (declaration: Declaration, featureFlags?: FeatureFlags): string[] => {
+const getExcludedRegularExpressions = (declaration: Declaration): string[] => {
   if ('excludedPattern' in declaration && declaration.excludedPattern) {
     const excludedPatterns: string[] = Array.isArray(declaration.excludedPattern)
       ? declaration.excludedPattern
@@ -241,19 +231,11 @@ const getExcludedRegularExpressions = (declaration: Declaration, featureFlags?:
       try {
         return parsePattern(excludedPattern)
       } catch (error: unknown) {
-        if (featureFlags?.edge_functions_fail_unsupported_regex) {
-          throw new Error(
+        throw wrapBundleError(
+          new Error(
             `Could not parse path declaration of function '${declaration.function}': ${(error as Error).message}`,
-          )
-        }
-
-        console.warn(
-          `Function '${
-            declaration.function
-          }' uses an unsupported regular expression and will therefore not be invoked: ${(error as Error).message}`,
+          ),
         )
-
-        return excludedPattern
       }
     })
   }

node/npm_import_error.ts

@@ -1,12 +1,8 @@
 class NPMImportError extends Error {
-  constructor(originalError: Error, moduleName: string, supportsNPM: boolean) {
-    let message = `It seems like you're trying to import an npm module. This is only supported via CDNs like esm.sh. Have you tried 'import mod from "https://esm.sh/${moduleName}"'?`
-
-    if (supportsNPM) {
-      message = `There was an error when loading the '${moduleName}' npm module. Support for npm modules in edge functions is an experimental feature. Refer to https://ntl.fyi/edge-functions-npm for more information.`
-    }
-
-    super(message)
+  constructor(originalError: Error, moduleName: string) {
+    super(
+      `There was an error when loading the '${moduleName}' npm module. Support for npm modules in edge functions is an experimental feature. Refer to https://ntl.fyi/edge-functions-npm for more information.`,
+    )
 
     this.name = 'NPMImportError'
     this.stack = originalError.stack
@@ -16,18 +12,18 @@ class NPMImportError extends Error {
   }
 }
 
-const wrapNpmImportError = (input: unknown, supportsNPM: boolean) => {
+const wrapNpmImportError = (input: unknown) => {
   if (input instanceof Error) {
     const match = input.message.match(/Relative import path "(.*)" not prefixed with/)
     if (match !== null) {
       const [, moduleName] = match
-      return new NPMImportError(input, moduleName, supportsNPM)
+      return new NPMImportError(input, moduleName)
     }
 
     const schemeMatch = input.message.match(/Error: Module not found "npm:(.*)"/)
     if (schemeMatch !== null) {
       const [, moduleName] = schemeMatch
-      return new NPMImportError(input, moduleName, supportsNPM)
+      return new NPMImportError(input, moduleName)
     }
   }
 

node/server/server.test.ts

@@ -24,9 +24,6 @@ test('Starts a server and serves requests for edge functions', async () => {
     importMapPaths,
     port,
     servePath,
-    featureFlags: {
-      edge_functions_npm_modules: true,
-    },
   })
 
   const functions = [

node/server/server.ts

@@ -38,7 +38,6 @@ const prepareServer = ({
   deno,
   distDirectory,
   distImportMapPath,
-  featureFlags,
   flags: denoFlags,
   formatExportTypeError,
   formatImportError,
@@ -71,21 +70,19 @@ const prepareServer = ({
     const importMap = baseImportMap.clone()
     const npmSpecifiersWithExtraneousFiles: string[] = []
 
-    if (featureFlags?.edge_functions_npm_modules) {
-      const vendor = await vendorNPMSpecifiers({
-        basePath,
-        directory: distDirectory,
-        functions: functions.map(({ path }) => path),
-        importMap,
-        logger,
-        referenceTypes: true,
-      })
-
-      if (vendor) {
-        features.npmModules = true
-        importMap.add(vendor.importMap)
-        npmSpecifiersWithExtraneousFiles.push(...vendor.npmSpecifiersWithExtraneousFiles)
-      }
+    const vendor = await vendorNPMSpecifiers({
+      basePath,
+      directory: distDirectory,
+      functions: functions.map(({ path }) => path),
+      importMap,
+      logger,
+      referenceTypes: true,
+    })
+
+    if (vendor) {
+      features.npmModules = true
+      importMap.add(vendor.importMap)
+      npmSpecifiersWithExtraneousFiles.push(...vendor.npmSpecifiersWithExtraneousFiles)
     }
 
     try {