From a32da856f822ae5ab6deb66316f2b43ca8a940e6 Mon Sep 17 00:00:00 2001 From: Netlify Team Account 1 <90322326+netlify-team-account-1@users.noreply.github.com> Date: Thu, 14 Oct 2021 10:19:12 +0200 Subject: [PATCH] feat: support functions within type=module packages (Closes #3394) (#3445) * feat: write reproduction test for #3394 * Update serving-functions.test.js * feat: fix by using import() for lambda funcs * chore: use esm: true for another test * feat: switch import/require based on Node version * chore: don't cache dynamic imports * fix: different approach trying to use "import" was surprisingly hard. I learned from @EduardoBoucas that we use esbuild (via ZISI) to transpile ESM to CJS. This commit improves detection for that, and makes sure that any upper-level "package.json: type=module" directive is overriden. * fix: optional chaining syntax unsupported * chore: use existing fs abstraction * chore: add explanatory comment * chore: rename variable Co-authored-by: Netlify Team Account 1 --- npm-shrinkwrap.json | 2 +- package.json | 1 + renovate.json5 | 1 + .../functions/runtimes/js/builders/zisi.js | 31 +++++++++-- tests/serving-functions.test.js | 52 +++++++++++++++---- tests/utils/site-builder.js | 5 +- 6 files changed, 73 insertions(+), 19 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index bc1a009d3f8..b44d978b3cf 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -106,6 +106,7 @@ "prettyjson": "^1.2.1", "pump": "^3.0.0", "raw-body": "^2.4.1", + "read-pkg-up": "^7.0.1", "resolve": "^1.12.0", "semver": "^7.3.4", "source-map-support": "^0.5.19", @@ -24366,7 +24367,6 @@ "resolved": "https://registry.npmjs.org/@oclif/command/-/command-1.8.0.tgz", "integrity": "sha512-5vwpq6kbvwkQwKqAoOU3L72GZ3Ta8RRrewKj9OJRolx28KLJJ8Dg9Rf7obRwt5jQA9bkYd8gqzMTrI7H3xLfaw==", "requires": { - "@oclif/config": "^1.15.1", "@oclif/errors": "^1.3.3", "@oclif/parser": "^3.8.3", "@oclif/plugin-help": "^3", diff --git a/package.json b/package.json index 0354127c5fa..4169e8caca4 100644 --- a/package.json +++ b/package.json @@ -173,6 +173,7 @@ "prettyjson": "^1.2.1", "pump": "^3.0.0", "raw-body": "^2.4.1", + "read-pkg-up": "^7.0.1", "resolve": "^1.12.0", "semver": "^7.3.4", "source-map-support": "^0.5.19", diff --git a/renovate.json5 b/renovate.json5 index 20c7db96367..e3d9f2d4e79 100644 --- a/renovate.json5 +++ b/renovate.json5 @@ -25,6 +25,7 @@ 'http-proxy-middleware', 'p-map', 'node-version-alias', + 'read-pkg-up', ], major: { enabled: false, diff --git a/src/lib/functions/runtimes/js/builders/zisi.js b/src/lib/functions/runtimes/js/builders/zisi.js index e3016912a93..7287900bcd1 100644 --- a/src/lib/functions/runtimes/js/builders/zisi.js +++ b/src/lib/functions/runtimes/js/builders/zisi.js @@ -3,9 +3,11 @@ const path = require('path') const { zipFunction } = require('@netlify/zip-it-and-ship-it') const decache = require('decache') const makeDir = require('make-dir') +const readPkgUp = require('read-pkg-up') const sourceMapSupport = require('source-map-support') const { NETLIFYDEVERR } = require('../../../../../utils/logo') +const { writeFileAsync } = require('../../../../fs') const { getPathInProject } = require('../../../../settings') const { normalizeFunctionsConfig } = require('../../../config') const { memoizedBuild } = require('../../../memoized-build') @@ -18,7 +20,7 @@ const addFunctionsConfigDefaults = (config) => ({ }, }) -const buildFunction = async ({ cache, config, directory, func, projectRoot, targetDirectory }) => { +const buildFunction = async ({ cache, config, directory, func, projectRoot, targetDirectory, hasTypeModule }) => { const zipOptions = { archiveFormat: 'none', basePath: projectRoot, @@ -42,6 +44,17 @@ const buildFunction = async ({ cache, config, directory, func, projectRoot, targ const srcFiles = inputs.filter((inputPath) => !inputPath.includes(`${path.sep}node_modules${path.sep}`)) const buildPath = path.join(functionPath, `${func.name}.js`) + // some projects include a package.json with "type=module", forcing Node to interpret every descending file + // as ESM. ZISI outputs CJS, so we emit an overriding directive into the output directory. + if (hasTypeModule) { + await writeFileAsync( + path.join(functionPath, `package.json`), + JSON.stringify({ + type: 'commonjs', + }), + ) + } + clearFunctionsCache(targetDirectory) return { buildPath, srcFiles } @@ -72,14 +85,22 @@ module.exports = async ({ config, directory, errorExit, func, projectRoot }) => normalizeFunctionsConfig({ functionsConfig: config.functions, projectRoot }), ) + const packageJson = await readPkgUp(func.mainFile) + const hasTypeModule = packageJson && packageJson.packageJson.type === 'module' + // We must use esbuild for certain file extensions. - const mustUseEsbuild = ['.mjs', '.ts'].includes(path.extname(func.mainFile)) + const mustTranspile = ['.mjs', '.ts'].includes(path.extname(func.mainFile)) + const mustUseEsbuild = hasTypeModule || mustTranspile + + if (mustUseEsbuild && !functionsConfig['*'].nodeBundler) { + functionsConfig['*'].nodeBundler = 'esbuild' + } // TODO: Resolve functions config globs so that we can check for the bundler // on a per-function basis. - const isUsingEsbuild = functionsConfig['*'].nodeBundler === 'esbuild_zisi' + const isUsingEsbuild = ['esbuild_zisi', 'esbuild'].includes(functionsConfig['*'].nodeBundler) - if (!mustUseEsbuild && !isUsingEsbuild) { + if (!isUsingEsbuild) { return false } @@ -90,7 +111,7 @@ module.exports = async ({ config, directory, errorExit, func, projectRoot }) => return { build: ({ cache = {} }) => - buildFunction({ cache, config: functionsConfig, directory, func, projectRoot, targetDirectory }), + buildFunction({ cache, config: functionsConfig, directory, func, projectRoot, targetDirectory, hasTypeModule }), builderName: 'zip-it-and-ship-it', target: targetDirectory, } diff --git a/tests/serving-functions.test.js b/tests/serving-functions.test.js index e97643bb0f2..ec6afade374 100644 --- a/tests/serving-functions.test.js +++ b/tests/serving-functions.test.js @@ -364,18 +364,13 @@ testMatrix.forEach(({ args }) => { const bundlerConfig = args.includes('esbuild') ? { node_bundler: 'esbuild' } : {} await builder - .withContentFile({ + .withFunction({ path: 'functions/help.ts', - content: ` -const handler = async () => { - return { - statusCode: 200, - body: 'I need somebody. Not just anybody.' - } -} - -export { handler } - `, + handler: async () => ({ + statusCode: 200, + body: 'I need somebody. Not just anybody.', + }), + esm: true, }) .withNetlifyToml({ config: { @@ -713,6 +708,41 @@ exports.handler = () => ({ }) }) }) + + test(testName('Serves functions inside a "type=module" package', args), async (t) => { + await withSiteBuilder('function-type-module', async (builder) => { + const bundlerConfig = args.includes('esbuild') ? { node_bundler: 'esbuild' } : {} + + await builder + .withNetlifyToml({ + config: { + build: { publish: 'public' }, + functions: { directory: 'functions' }, + ...bundlerConfig, + }, + }) + .withPackageJson({ + packageJson: { + type: 'module', + }, + }) + .withFunction({ + path: 'hello.js', + handler: async () => ({ + statusCode: 200, + body: 'hello from es module!', + }), + esm: true, + }) + .buildAsync() + + await withDevServer({ cwd: builder.directory, args }, async ({ port, outputBuffer }) => { + await tryAndLogOutput(async () => { + t.is(await got(`http://localhost:${port}/.netlify/functions/hello`).text(), 'hello from es module!') + }, outputBuffer) + }) + }) + }) }) test('Serves functions that dynamically load files included in the `functions.included_files` config property', async (t) => { diff --git a/tests/utils/site-builder.js b/tests/utils/site-builder.js index 4f5b4472975..00f7ebd77e0 100644 --- a/tests/utils/site-builder.js +++ b/tests/utils/site-builder.js @@ -55,11 +55,12 @@ const createSiteBuilder = ({ siteName }) => { }) return builder }, - withFunction: ({ pathPrefix = 'functions', path: filePath, handler }) => { + withFunction: ({ pathPrefix = 'functions', path: filePath, handler, esm = false }) => { const dest = path.join(directory, pathPrefix, filePath) tasks.push(async () => { await ensureDir(path.dirname(dest)) - await fs.writeFileAsync(dest, `exports.handler = ${handler.toString()}`) + const file = esm ? `export const handler = ${handler.toString()}` : `exports.handler = ${handler.toString()}` + await fs.writeFileAsync(dest, file) }) return builder },