diff --git a/packages/edge-bundler/node/bundler.ts b/packages/edge-bundler/node/bundler.ts index a89a2b86ee..4ac06446e4 100644 --- a/packages/edge-bundler/node/bundler.ts +++ b/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) diff --git a/packages/edge-bundler/node/npm_dependencies.ts b/packages/edge-bundler/node/npm_dependencies.ts index d97071e138..087ef2ab6a 100644 --- a/packages/edge-bundler/node/npm_dependencies.ts +++ b/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 => { + 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 => { + 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 => { + 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() + const npmSpecifiers: { specifier: string; types?: string }[] = [] const npmSpecifiersWithExtraneousFiles = new Set() - 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,6 +200,7 @@ interface VendorNPMSpecifiersOptions { functions: string[] importMap: ImportMap logger: Logger + referenceTypes: boolean } export const vendorNPMSpecifiers = async ({ @@ -130,6 +208,7 @@ export const vendorNPMSpecifiers = async ({ 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,10 +225,11 @@ 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 } @@ -157,13 +237,14 @@ export const vendorNPMSpecifiers = async ({ // 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) @@ -171,7 +252,7 @@ export const vendorNPMSpecifiers = async ({ // 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 = `/// \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. diff --git a/packages/edge-bundler/node/server/server.test.ts b/packages/edge-bundler/node/server/server.test.ts index ee92c6b729..502314da6b 100644 --- a/packages/edge-bundler/node/server/server.test.ts +++ b/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( + `/// `, + ) + + const identidadeBarrelFile = await readFile(join(servePath, 'barrel-2.js'), 'utf-8') + expect(identidadeBarrelFile).toContain( + `/// `, + ) }) diff --git a/packages/edge-bundler/node/server/server.ts b/packages/edge-bundler/node/server/server.ts index ef22ffc9b8..d77c69852c 100644 --- a/packages/edge-bundler/node/server/server.ts +++ b/packages/edge-bundler/node/server/server.ts @@ -78,6 +78,7 @@ const prepareServer = ({ functions: functions.map(({ path }) => path), importMap, logger, + referenceTypes: true, }) if (vendor) { diff --git a/packages/edge-bundler/test/fixtures/serve_test/.netlify/.gitignore b/packages/edge-bundler/test/fixtures/serve_test/.netlify/.gitignore new file mode 100644 index 0000000000..0829263618 --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/.netlify/.gitignore @@ -0,0 +1 @@ +edge-functions-serve \ No newline at end of file diff --git a/packages/edge-bundler/test/fixtures/serve_test/netlify/edge-functions/echo_env.ts b/packages/edge-bundler/test/fixtures/serve_test/netlify/edge-functions/echo_env.ts index 3ebb05a1d8..1ed20a62bb 100644 --- a/packages/edge-bundler/test/fixtures/serve_test/netlify/edge-functions/echo_env.ts +++ b/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', diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/@pt-committee/identidade/index.js b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@pt-committee/identidade/index.js new file mode 100644 index 0000000000..8651210ecc --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@pt-committee/identidade/index.js @@ -0,0 +1 @@ +module.exports = (v) => v diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/@pt-committee/identidade/package.json b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@pt-committee/identidade/package.json new file mode 100644 index 0000000000..b4cc9144d2 --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@pt-committee/identidade/package.json @@ -0,0 +1 @@ +{ "name": "@pt-committee/identidade" } \ No newline at end of file diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/@types/pt-committee__identidade/index.d.ts b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@types/pt-committee__identidade/index.d.ts new file mode 100644 index 0000000000..1161e7cc4a --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@types/pt-committee__identidade/index.d.ts @@ -0,0 +1,3 @@ +declare function id(v: T): T + +export default id diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/@types/pt-committee__identidade/package.json b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@types/pt-committee__identidade/package.json new file mode 100644 index 0000000000..7a10617da1 --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/@types/pt-committee__identidade/package.json @@ -0,0 +1,4 @@ +{ + "name": "@types/pt-committee__identidade", + "typings": "index.d.ts" +} \ No newline at end of file diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/index.js b/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/index.js new file mode 100644 index 0000000000..55023d8926 --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/index.js @@ -0,0 +1,6 @@ +import { readFileSync } from 'node:fs' + +export function getWords() { + const wordsFile = readFileSync(new URL('./words.txt', import.meta.url), { encoding: "utf-8" }) + return wordsFile.split('\n') +} diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/package.json b/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/package.json new file mode 100644 index 0000000000..b356768235 --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/package.json @@ -0,0 +1,5 @@ +{ + "name": "dictionary", + "type": "module", + "main": "index.js" +} \ No newline at end of file diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/words.txt b/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/words.txt new file mode 100644 index 0000000000..4f060fe22f --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/dictionary/words.txt @@ -0,0 +1,18 @@ +Flibberdegibbet +Quibble +Gobbledygook +Wobblebonk +Blibber-blabber +Malarkey +Flibbertigibbet +Gobbledyguke +Jibber-jabber +Gibberish +Noodlehead +Snickersnee +Skedaddle +Lollygag +Hootenanny +Razzmatazz +Higgledy-piggledy +Wobblegong \ No newline at end of file diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/index.js b/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/index.js new file mode 100644 index 0000000000..7824982b19 --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/index.js @@ -0,0 +1 @@ +export default (v) => v diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/package.json b/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/package.json new file mode 100644 index 0000000000..cec4327efb --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/package.json @@ -0,0 +1,6 @@ +{ + "name": "id", + "main": "index.js", + "type": "module", + "types": "types.d.ts" +} \ No newline at end of file diff --git a/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/types.d.ts b/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/types.d.ts new file mode 100644 index 0000000000..1161e7cc4a --- /dev/null +++ b/packages/edge-bundler/test/fixtures/serve_test/node_modules/id/types.d.ts @@ -0,0 +1,3 @@ +declare function id(v: T): T + +export default id