Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

feat: use esbuild by default for es modules, if detected #625

Merged
merged 15 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ These are supplied to each of the entrypoint functions (`zipFunction`, `zipFunct
`listFunctionsFiles`) as a named parameter called `featureFlags`. It consists of an object where each key is the name of
a feature flag and the values are Booleans indicating whether each feature flag is enabled or disabled.

The list of all feature flags currently being used can be found at [here](src/feature_flags.js).
The list of all feature flags currently being used can be found [here](src/feature_flags.js).
charliegroll marked this conversation as resolved.
Show resolved Hide resolved

# Troubleshooting

Expand Down
11 changes: 11 additions & 0 deletions package-lock.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 @@ -63,6 +63,7 @@
"del": "^6.0.0",
"elf-cam": "^0.1.1",
"end-of-stream": "^1.4.4",
"es-module-lexer": "^0.7.1",
ehmicky marked this conversation as resolved.
Show resolved Hide resolved
"execa": "^5.0.0",
"filter-obj": "^2.0.1",
"find-up": "^5.0.0",
Expand Down
1 change: 1 addition & 0 deletions src/feature_flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { env } = require('process')
const FLAGS = {
buildGoSource: Boolean(env.NETLIFY_EXPERIMENTAL_BUILD_GO_SOURCE),
buildRustSource: Boolean(env.NETLIFY_EXPERIMENTAL_BUILD_RUST_SOURCE),
defaultEsModulesToEsbuild: Boolean(env.NETLIFY_EXPERIMENTAL_DEFAULT_ES_MODULES_TO_ESBUILD),
}

const getFlags = (input = {}, flags = FLAGS) =>
Expand Down
24 changes: 24 additions & 0 deletions src/runtimes/node/detect_es_module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const { readFile } = require('fs')
const { promisify } = require('util')

const pReadFile = promisify(readFile)

const { init, parse } = require('es-module-lexer')
Copy link
Member

Choose a reason for hiding this comment

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

We already have a JavaScript parser (acorn) as a dependency, which should be able to tell us whether a file uses ESM or not. If it is, I would try to use it before adding a new dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

sure sure, I'll take a look 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering about the performance difference there considering:

  • acorn is implemented in JavaScript while es-module-lexer is in WebAssembly
  • acorn does lexing+parsing while es-module-lexer does lexing only
  • acorn parses all possible JavaScript syntax while es-module-lexer only looks for imports/exports

The author of es-module-lexer actually claims the following:

For an example of the performance, Angular 1 (720KiB) is fully parsed in 5ms, in comparison to the fastest JS parser, Acorn which takes over 100ms.

Copy link
Member Author

Choose a reason for hiding this comment

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

I second @ehmicky 's evaluation - we need a parser and that's just not what es-module-lexer provides.

that being said, es-module-lexer does have a very light footprint... if we want, I can look into how to accomplish this with acorn, but we can't replace acorn with this

zip-it-and-ship-it on  cg2/617/DetectEsModules [$] is 📦 v4.20.0 via  v16.7.0 
❯ du -ha node_modules/es-module-lexer
4.0K    node_modules/es-module-lexer/types/lexer.d.ts
4.0K    node_modules/es-module-lexer/types
4.0K    node_modules/es-module-lexer/LICENSE
4.0K    node_modules/es-module-lexer/CHANGELOG.md
 12K    node_modules/es-module-lexer/dist/lexer.cjs
 12K    node_modules/es-module-lexer/dist/lexer.js
 24K    node_modules/es-module-lexer/dist
8.0K    node_modules/es-module-lexer/README.md
4.0K    node_modules/es-module-lexer/package.json
 48K    node_modules/es-module-lexer


const detectEsModule = async ({ mainFile }) => {
if (!mainFile) {
charliegroll marked this conversation as resolved.
Show resolved Hide resolved
return false
}

try {
const [mainFileContents] = await Promise.all([pReadFile(mainFile, 'utf8'), init])
const [imports, exports] = parse(mainFileContents)

return imports.length !== 0 || exports.length !== 0
} catch {
// If there are any problems with init or parsing, assume it's not an ES module
return false
}
}

module.exports = { detectEsModule }
26 changes: 20 additions & 6 deletions src/runtimes/node/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,34 @@ const cpFile = require('cp-file')

const { JS_BUNDLER_ESBUILD, JS_BUNDLER_ESBUILD_ZISI, JS_BUNDLER_ZISI, RUNTIME_JS } = require('../../utils/consts')

const { detectEsModule } = require('./detect_es_module')
const { findFunctionsInPaths } = require('./finder')
const { getSrcFiles } = require('./src_files')
const { zipEsbuild } = require('./zip_esbuild')
const { zipZisi } = require('./zip_zisi')

// We use ZISI as the default bundler, except for certain extensions, for which
// esbuild is the only option.
const getDefaultBundler = ({ extension }) =>
['.mjs', '.ts'].includes(extension) ? JS_BUNDLER_ESBUILD : JS_BUNDLER_ZISI
const getDefaultBundler = async ({ extension, mainFile, featureFlags = {} }) => {
if (['.mjs', '.ts'].includes(extension)) {
return JS_BUNDLER_ESBUILD
}

if (featureFlags.defaultEsModulesToEsbuild) {
const isEsModule = await detectEsModule({ mainFile })

if (isEsModule) {
return JS_BUNDLER_ESBUILD
}
}

return JS_BUNDLER_ZISI
}

// A proxy for the `getSrcFiles` function which adds a default `bundler` using
// the `getDefaultBundler` function.
const getSrcFilesWithBundler = (parameters) => {
const bundler = parameters.config.nodeBundler || getDefaultBundler({ extension: parameters.extension })
const getSrcFilesWithBundler = async (parameters) => {
const bundler = parameters.config.nodeBundler || (await getDefaultBundler({ extension: parameters.extension }))

return getSrcFiles({ ...parameters, bundler })
}
Expand All @@ -35,9 +49,9 @@ const zipFunction = async function ({
srcDir,
srcPath,
stat,
featureFlags,
}) {
const bundler = config.nodeBundler || getDefaultBundler({ extension })

const bundler = config.nodeBundler || (await getDefaultBundler({ extension, mainFile, featureFlags }))
// If the file is a zip, we assume the function is bundled and ready to go.
// We simply copy it to the destination path with no further processing.
if (extension === '.zip') {
Expand Down
2 changes: 2 additions & 0 deletions src/zip.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ const zipFunctions = async function (
srcDir: func.srcDir,
srcPath: func.srcPath,
stat: func.stat,
featureFlags,
})

return { ...zipResult, mainFile: func.mainFile, name: func.name, runtime: func.runtime }
Expand Down Expand Up @@ -151,6 +152,7 @@ const zipFunction = async function (
stat,
runtime,
pluginsModulesPath,
featureFlags,
})

return formatZipResult({ ...zipResult, mainFile, name, runtime })
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// eslint-disable-next-line node/no-unsupported-features/es-syntax
export const howdy = 'Yee haw!'
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// eslint-disable-next-line node/no-unsupported-features/es-syntax
import getZero from '../function/file'

getZero()
35 changes: 29 additions & 6 deletions tests/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,16 +272,39 @@ testBundlers('Can use dynamic import() with esbuild', [ESBUILD, ESBUILD_ZISI], a
await zipNode(t, 'dynamic-import', { opts: { config: { '*': { nodeBundler: bundler } } } })
})

testBundlers('Bundling does not crash with dynamic import() with zisi', [DEFAULT], async (bundler, t) => {
await t.throwsAsync(zipNode(t, 'dynamic-import', { opts: { config: { '*': { nodeBundler: bundler } } } }), {
Copy link
Member Author

Choose a reason for hiding this comment

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

this is no longer a reachable flow, since we'll detect imports/exports and default to esbuild

message: /export/,
})
})

testBundlers('Can require local files', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
await zipNode(t, 'local-require', { opts: { config: { '*': { nodeBundler: bundler } } } })
})

testBundlers(
'Can bundle functions with `.js` extension using ES Modules and feature flag ON',
[ESBUILD, ESBUILD_ZISI, DEFAULT],
async (bundler, t) => {
await zipNode(t, 'local-require-esm', {
length: 3,
opts: { featureFlags: { defaultEsModulesToEsbuild: true }, config: { '*': { nodeBundler: bundler } } },
})
},
)

testBundlers(
'Can bundle functions with `.js` extension using ES Modules and feature flag OFF',
[ESBUILD, ESBUILD_ZISI, DEFAULT],
async (bundler, t) => {
await (bundler === DEFAULT
? t.throwsAsync(
zipNode(t, 'local-require-esm', {
length: 3,
opts: { featureFlags: { defaultEsModulesToEsbuild: false }, config: { '*': { nodeBundler: bundler } } },
}),
)
: zipNode(t, 'local-require-esm', {
length: 3,
opts: { featureFlags: { defaultEsModulesToEsbuild: false }, config: { '*': { nodeBundler: bundler } } },
}))
},
)

testBundlers('Can require local files deeply', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {
await zipNode(t, 'local-deep-require', { opts: { config: { '*': { nodeBundler: bundler } } } })
})
Expand Down