feat: detect Typescript typings for NPM modules and reference them fr…

…om barrel files (netlify/edge-bundler#505)

* chore: add npm module to serve test

* feat: detect `types` from package.json and prepend it in typescript directive

* fix: detect @types packages as well

* fix: respect scoped packages

* chore: add comment

* Update node/npm_dependencies.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* chore: address some feedback

* fix: make type reference relative path

* fix: only look at `package.json` files

* fix: windows test

* chore: address feedback

* fix: detect extraneous files

* refactor: rename variables

* refactor: break things up into two functions

* refactor: dont rename variable

* fix: write everything in one go

* refactor: scrap the file descriptor

---------

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

packages/edge-bundler/node/bundler.ts

@@ -272,6 +272,7 @@ const safelyVendorNPMSpecifiers = async ({
       functions: functions.map(({ path }) => path),
       importMap,
       logger,
+      referenceTypes: false,
     })
   } catch (error) {
     logger.system(error)

packages/edge-bundler/node/npm_dependencies.ts

@@ -6,6 +6,7 @@ import { fileURLToPath, pathToFileURL } from 'url'
 import { resolve, ParsedImportMap } from '@import-maps/resolve'
 import { nodeFileTrace, resolve as nftResolve } from '@vercel/nft'
 import { build } from 'esbuild'
+import { findUp } from 'find-up'
 import getPackageName from 'get-package-name'
 import tmp from 'tmp-promise'
 
@@ -14,6 +15,66 @@ import { Logger } from './logger.js'
 
 const TYPESCRIPT_EXTENSIONS = new Set(['.ts', '.cts', '.mts'])
 
+/**
+ * Returns the name of the `@types/` package used by a given specifier. Most of
+ * the times this is just the specifier itself, but scoped packages suffer a
+ * transformation (e.g. `@netlify/functions` -> `@types/netlify__functions`).
+ * https://github.com/DefinitelyTyped/DefinitelyTyped#what-about-scoped-packages
+ */
+const getTypesPackageName = (specifier: string) => {
+  if (!specifier.startsWith('@')) return path.join('@types', specifier)
+  const [scope, pkg] = specifier.split('/')
+  return path.join('@types', `${scope.replace('@', '')}__${pkg}`)
+}
+
+/**
+ * Finds corresponding DefinitelyTyped packages (`@types/...`) and returns path to declaration file.
+ */
+const getTypePathFromTypesPackage = async (
+  packageName: string,
+  packageJsonPath: string,
+): Promise<string | undefined> => {
+  const typesPackagePath = await findUp(`node_modules/${getTypesPackageName(packageName)}/package.json`, {
+    cwd: packageJsonPath,
+  })
+  if (!typesPackagePath) {
+    return undefined
+  }
+
+  const { types, typings } = JSON.parse(await fs.readFile(typesPackagePath, 'utf8'))
+  const declarationPath = types ?? typings
+  if (typeof declarationPath === 'string') {
+    return path.join(typesPackagePath, '..', declarationPath)
+  }
+
+  return undefined
+}
+
+/**
+ * Starting from a `package.json` file, this tries detecting a TypeScript declaration file.
+ * It first looks at the `types` and `typings` fields in `package.json`.
+ * If it doesn't find them, it falls back to DefinitelyTyped packages (`@types/...`).
+ */
+const getTypesPath = async (packageJsonPath: string): Promise<string | undefined> => {
+  const { name, types, typings } = JSON.parse(await fs.readFile(packageJsonPath, 'utf8'))
+  // this only looks at `.types` and `.typings` fields. there might also be data in `exports -> . -> types -> import/default`.
+  // we're ignoring that for now.
+  const declarationPath = types ?? typings
+  if (typeof declarationPath === 'string') {
+    return path.join(packageJsonPath, '..', declarationPath)
+  }
+
+  return await getTypePathFromTypesPackage(name, packageJsonPath)
+}
+
+const safelyDetectTypes = async (packageJsonPath: string): Promise<string | undefined> => {
+  try {
+    return await getTypesPath(packageJsonPath)
+  } catch {
+    return undefined
+  }
+}
+
 // Workaround for https://github.com/evanw/esbuild/issues/1921.
 const banner = {
   js: `
@@ -33,8 +94,14 @@ const banner = {
  * @param basePath Root of the project
  * @param functions Functions to parse
  * @param importMap Import map to apply when resolving imports
+ * @param referenceTypes Whether to detect typescript declarations and reference them in the output
  */
-const getNPMSpecifiers = async (basePath: string, functions: string[], importMap: ParsedImportMap) => {
+const getNPMSpecifiers = async (
+  basePath: string,
+  functions: string[],
+  importMap: ParsedImportMap,
+  referenceTypes: boolean,
+) => {
   const baseURL = pathToFileURL(basePath)
   const { reasons } = await nodeFileTrace(functions, {
     base: basePath,
@@ -72,28 +139,11 @@ const getNPMSpecifiers = async (basePath: string, functions: string[], importMap
       return nftResolve(specifier, ...args)
     },
   })
-  const npmSpecifiers = new Set<string>()
+  const npmSpecifiers: { specifier: string; types?: string }[] = []
   const npmSpecifiersWithExtraneousFiles = new Set<string>()
 
-  reasons.forEach((reason, filePath) => {
-    const packageName = getPackageName(filePath)
-
-    if (packageName === undefined) {
-      return
-    }
-
+  for (const [filePath, reason] of reasons.entries()) {
     const parents = [...reason.parents]
-    const isDirectDependency = parents.some((parentPath) => !parentPath.startsWith(`node_modules${path.sep}`))
-
-    // We're only interested in capturing the specifiers that are first-level
-    // dependencies. Because we'll bundle all modules in a subsequent step,
-    // any transitive dependencies will be handled then.
-    if (isDirectDependency) {
-      const specifier = getPackageName(filePath)
-
-      npmSpecifiers.add(specifier)
-    }
-
     const isExtraneousFile = reason.type.every((type) => type === 'asset')
 
     // An extraneous file is a dependency that was traced by NFT and marked
@@ -109,10 +159,37 @@ const getNPMSpecifiers = async (basePath: string, functions: string[], importMap
         }
       })
     }
-  })
+
+    // every dependency will have its `package.json` in `reasons` exactly once.
+    // by only looking at this file, we save us from doing duplicate work.
+    const isPackageJson = filePath.endsWith('package.json')
+    if (!isPackageJson) continue
+
+    const packageName = getPackageName(filePath)
+    if (packageName === undefined) continue
+
+    const isDirectDependency = parents.some((parentPath) => {
+      if (!parentPath.startsWith(`node_modules${path.sep}`)) return true
+      // typically, edge functions have no direct dependency on the `package.json` of a module.
+      // it's the impl files that depend on `package.json`, so we need to check the parents of
+      // the `package.json` file as well to see if the module is a direct dependency.
+      const parents = [...(reasons.get(parentPath)?.parents ?? [])]
+      return parents.some((parentPath) => !parentPath.startsWith(`node_modules${path.sep}`))
+    })
+
+    // We're only interested in capturing the specifiers that are first-level
+    // dependencies. Because we'll bundle all modules in a subsequent step,
+    // any transitive dependencies will be handled then.
+    if (isDirectDependency) {
+      npmSpecifiers.push({
+        specifier: packageName,
+        types: referenceTypes ? await safelyDetectTypes(path.join(basePath, filePath)) : undefined,
+      })
+    }
+  }
 
   return {
-    npmSpecifiers: [...npmSpecifiers],
+    npmSpecifiers,
     npmSpecifiersWithExtraneousFiles: [...npmSpecifiersWithExtraneousFiles],
   }
 }
@@ -123,13 +200,15 @@ interface VendorNPMSpecifiersOptions {
   functions: string[]
   importMap: ImportMap
   logger: Logger
+  referenceTypes: boolean
 }
 
 export const vendorNPMSpecifiers = async ({
   basePath,
   directory,
   functions,
   importMap,
+  referenceTypes,
 }: VendorNPMSpecifiersOptions) => {
   // The directories that esbuild will use when resolving Node modules. We must
   // set these manually because esbuild will be operating from a temporary
@@ -146,32 +225,34 @@ export const vendorNPMSpecifiers = async ({
     basePath,
     functions,
     importMap.getContentsWithURLObjects(),
+    referenceTypes,
   )
 
   // If we found no specifiers, there's nothing left to do here.
-  if (npmSpecifiers.length === 0) {
+  if (Object.keys(npmSpecifiers).length === 0) {
     return
   }
 
   // To bundle an entire module and all its dependencies, create a barrel file
   // where we re-export everything from that specifier. We do this for every
   // specifier, and each of these files will become entry points to esbuild.
   const ops = await Promise.all(
-    npmSpecifiers.map(async (specifier, index) => {
+    npmSpecifiers.map(async ({ specifier, types }, index) => {
       const code = `import * as mod from "${specifier}"; export default mod.default; export * from "${specifier}";`
-      const filePath = path.join(temporaryDirectory.path, `barrel-${index}.js`)
+      const barrelName = `barrel-${index}.js`
+      const filePath = path.join(temporaryDirectory.path, barrelName)
 
       await fs.writeFile(filePath, code)
 
-      return { filePath, specifier }
+      return { filePath, specifier, barrelName, types }
     }),
   )
   const entryPoints = ops.map(({ filePath }) => filePath)
 
   // Bundle each of the barrel files we created. We'll end up with a compiled
   // version of each of the barrel files, plus any chunks of shared code
   // between them (such that a common module isn't bundled twice).
-  await build({
+  const { outputFiles } = await build({
     allowOverwrite: true,
     banner,
     bundle: true,
@@ -183,8 +264,20 @@ export const vendorNPMSpecifiers = async ({
     platform: 'node',
     splitting: true,
     target: 'es2020',
+    write: false,
   })
 
+  await Promise.all(
+    outputFiles.map(async (file) => {
+      const types = ops.find((op) => file.path.endsWith(op.barrelName))?.types
+      let content = file.text
+      if (types) {
+        content = `/// <reference types="${path.relative(file.path, types)}" />\n${content}`
+      }
+      await fs.writeFile(file.path, content)
+    }),
+  )
+
   // Add all Node.js built-ins to the import map, so any unprefixed specifiers
   // (e.g. `process`) resolve to the prefixed versions (e.g. `node:prefix`),
   // which Deno can process.

packages/edge-bundler/node/server/server.test.ts

@@ -1,8 +1,8 @@
+import { readFile } from 'fs/promises'
 import { join } from 'path'
 
 import getPort from 'get-port'
 import fetch from 'node-fetch'
-import { tmpName } from 'tmp-promise'
 import { v4 as uuidv4 } from 'uuid'
 import { test, expect } from 'vitest'
 
@@ -17,13 +17,16 @@ test('Starts a server and serves requests for edge functions', async () => {
   }
   const port = await getPort()
   const importMapPaths = [join(paths.internal, 'import_map.json'), join(paths.user, 'import-map.json')]
-  const servePath = await tmpName()
+  const servePath = join(basePath, '.netlify', 'edge-functions-serve')
   const server = await serve({
     basePath,
     bootstrapURL: 'https://edge.netlify.com/bootstrap/index-combined.ts',
     importMapPaths,
     port,
     servePath,
+    featureFlags: {
+      edge_functions_npm_modules: true,
+    },
   })
 
   const functions = [
@@ -44,16 +47,17 @@ test('Starts a server and serves requests for edge functions', async () => {
     getFunctionsConfig: true,
   }
 
-  const { features, functionsConfig, graph, success } = await server(
+  const { features, functionsConfig, graph, success, npmSpecifiersWithExtraneousFiles } = await server(
     functions,
     {
       very_secret_secret: 'i love netlify',
     },
     options,
   )
-  expect(features).toEqual({})
+  expect(features).toEqual({ npmModules: true })
   expect(success).toBe(true)
   expect(functionsConfig).toEqual([{ path: '/my-function' }, {}, { path: '/global-netlify' }])
+  expect(npmSpecifiersWithExtraneousFiles).toEqual(['dictionary'])
 
   for (const key in functions) {
     const graphEntry = graph?.modules.some(
@@ -95,4 +99,22 @@ test('Starts a server and serves requests for edge functions', async () => {
     global: 'i love netlify',
     local: 'i love netlify',
   })
+
+  const idBarrelFile = await readFile(join(servePath, 'barrel-0.js'), 'utf-8')
+  expect(idBarrelFile).toContain(
+    `/// <reference types="${join('..', '..', '..', 'node_modules', 'id', 'types.d.ts')}" />`,
+  )
+
+  const identidadeBarrelFile = await readFile(join(servePath, 'barrel-2.js'), 'utf-8')
+  expect(identidadeBarrelFile).toContain(
+    `/// <reference types="${join(
+      '..',
+      '..',
+      '..',
+      'node_modules',
+      '@types',
+      'pt-committee__identidade',
+      'index.d.ts',
+    )}" />`,
+  )
 })

packages/edge-bundler/node/server/server.ts

@@ -78,6 +78,7 @@ const prepareServer = ({
         functions: functions.map(({ path }) => path),
         importMap,
         logger,
+        referenceTypes: true,
       })
 
       if (vendor) {

packages/edge-bundler/test/fixtures/serve_test/.netlify/.gitignore

@@ -0,0 +1 @@
+edge-functions-serve

packages/edge-bundler/test/fixtures/serve_test/netlify/edge-functions/echo_env.ts

@@ -1,8 +1,20 @@
 import { Config } from 'https://edge.netlify.com'
 
 import { yell } from 'helper'
+import id from 'id'
+import identidade from '@pt-committee/identidade'
 
-export default () => new Response(yell(Deno.env.get('very_secret_secret') ?? ''))
+import { getWords } from 'dictionary'
+
+// this will throw since FS access is not available in Edge Functions.
+// but we need this line so that `dictionary` is scanned for extraneous dependencies
+try {
+  getWords()
+} catch {}
+
+export default () => {
+  return new Response(yell(identidade(id(Deno.env.get('very_secret_secret'))) ?? ''))
+}
 
 export const config: Config = {
   path: '/my-function',