Skip to content

Commit

Permalink
feat: support functions within type=module packages (Closes #3394) (#…
Browse files Browse the repository at this point in the history
…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 <netlify-team-account-1@users.noreply.github.com>
  • Loading branch information
1 parent 7f85116 commit a32da85
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 19 deletions.
2 changes: 1 addition & 1 deletion npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
'http-proxy-middleware',
'p-map',
'node-version-alias',
'read-pkg-up',
],
major: {
enabled: false,
Expand Down
31 changes: 26 additions & 5 deletions src/lib/functions/runtimes/js/builders/zisi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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,
Expand All @@ -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 }
Expand Down Expand Up @@ -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
}

Expand All @@ -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,
}
Expand Down
52 changes: 41 additions & 11 deletions tests/serving-functions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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) => {
Expand Down
5 changes: 3 additions & 2 deletions tests/utils/site-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down

1 comment on commit a32da85

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Benchmark results

Package size: 357 MB

Please sign in to comment.