Skip to content

Commit

Permalink
feat: replace precinct with esbuild (#659)
Browse files Browse the repository at this point in the history
* refactor: replace precinct with esbuild

* refactor: add feature flag

* fix: add feature flags object

* refactor: turn feature flag off by default

* chore: add test

* refactor: use intermediate variable

* refactor: await `safeUnlink` call

Co-authored-by: ehmicky <ehmicky@users.noreply.github.com>

Co-authored-by: ehmicky <ehmicky@users.noreply.github.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed Sep 20, 2021
1 parent c28eaaf commit 5bf51d0
Show file tree
Hide file tree
Showing 16 changed files with 177 additions and 30 deletions.
7 changes: 2 additions & 5 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
Expand Up @@ -68,6 +68,7 @@
"filter-obj": "^2.0.1",
"find-up": "^5.0.0",
"glob": "^7.1.6",
"is-builtin-module": "^3.1.0",
"junk": "^3.1.0",
"locate-path": "^6.0.0",
"make-dir": "^3.1.0",
Expand Down
1 change: 1 addition & 0 deletions src/feature_flags.js
Expand Up @@ -5,6 +5,7 @@ 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),
parseWithEsbuild: false,
}

const getFlags = (input = {}, flags = FLAGS) =>
Expand Down
7 changes: 5 additions & 2 deletions src/main.js
Expand Up @@ -27,7 +27,7 @@ const listFunctionsFiles = async function (relativeSrcFolders, { config, feature
getPluginsModulesPath(srcFolders[0]),
])
const listedFunctionsFiles = await Promise.all(
[...functions.values()].map((func) => getListedFunctionFiles(func, { pluginsModulesPath })),
[...functions.values()].map((func) => getListedFunctionFiles(func, { featureFlags, pluginsModulesPath })),
)

// TODO: switch to Array.flat() once we drop support for Node.js < 11.0.0
Expand All @@ -41,9 +41,10 @@ const getListedFunction = function ({ runtime, name, mainFile, extension }) {

const getListedFunctionFiles = async function (
{ config, runtime, name, stat, mainFile, extension, srcPath, srcDir },
{ pluginsModulesPath },
{ featureFlags, pluginsModulesPath },
) {
const srcFiles = await getSrcFiles({
featureFlags,
runtime,
stat,
mainFile,
Expand All @@ -59,6 +60,7 @@ const getListedFunctionFiles = async function (
const getSrcFiles = function ({
bundler,
config,
featureFlags,
runtime,
stat,
mainFile,
Expand All @@ -77,6 +79,7 @@ const getSrcFiles = function ({
bundler,
config,
extension,
featureFlags,
srcPath,
mainFile,
srcDir,
Expand Down
67 changes: 49 additions & 18 deletions src/node_dependencies/index.js
Expand Up @@ -4,6 +4,8 @@ const findUp = require('find-up')
const { not: notJunk } = require('junk')
const precinct = require('precinct')

const { listImports } = require('../runtimes/node/list_imports')

const { getPackageJson } = require('./package_json')
const { resolvePathPreserveSymlinks } = require('./resolve')
const { getExternalAndIgnoredModulesFromSpecialCases } = require('./special_cases')
Expand All @@ -23,10 +25,17 @@ const getPluginsModulesPath = (srcDir) => findUp(`${AUTO_PLUGINS_DIR}node_module
// Retrieve the paths to the Node.js files to zip.
// We only include the files actually needed by the function because AWS Lambda
// has a size limit for the zipped file. It also makes cold starts faster.
const listFilesUsingLegacyBundler = async function ({ srcPath, mainFile, srcDir, stat, pluginsModulesPath }) {
const listFilesUsingLegacyBundler = async function ({
featureFlags,
srcPath,
mainFile,
srcDir,
stat,
pluginsModulesPath,
}) {
const [treeFiles, depFiles] = await Promise.all([
getTreeFiles(srcPath, stat),
getDependencies(mainFile, srcDir, pluginsModulesPath),
getDependencies(mainFile, srcDir, pluginsModulesPath, featureFlags),
])
const files = [...treeFiles, ...depFiles].map(normalize)
const uniqueFiles = [...new Set(files)]
Expand All @@ -43,36 +52,48 @@ const isNotJunk = function (file) {
}

// Retrieve all the files recursively required by a Node.js file
const getDependencies = async function (mainFile, srcDir, pluginsModulesPath) {
const getDependencies = async function (mainFile, srcDir, pluginsModulesPath, featureFlags) {
const packageJson = await getPackageJson(srcDir)
const state = getNewCache()

try {
return await getFileDependencies({ path: mainFile, packageJson, state, pluginsModulesPath })
return await getFileDependencies({ featureFlags, path: mainFile, packageJson, pluginsModulesPath, state })
} catch (error) {
error.message = `In file "${mainFile}"\n${error.message}`
throw error
}
}

const getFileDependencies = async function ({ path, packageJson, pluginsModulesPath, state, treeShakeNext }) {
const getFileDependencies = async function ({
featureFlags,
path,
packageJson,
pluginsModulesPath,
state,
treeShakeNext,
}) {
if (state.localFiles.has(path)) {
return []
}

state.localFiles.add(path)

const basedir = dirname(path)
// This parses JavaScript in `path` to retrieve all the `require()` statements
// TODO: `precinct.paperwork()` uses `fs.readFileSync()` under the hood,
// but should use `fs.readFile()` instead
const dependencies = precinct.paperwork(path, { includeCore: false })
const dependencies = featureFlags.parseWithEsbuild
? await listImports({ path })
: precinct.paperwork(path, { includeCore: false })
const depsPaths = await Promise.all(
dependencies
.filter(Boolean)
.map((dependency) =>
getImportDependencies({ dependency, basedir, packageJson, state, treeShakeNext, pluginsModulesPath }),
),
dependencies.filter(Boolean).map((dependency) =>
getImportDependencies({
dependency,
basedir,
featureFlags,
packageJson,
pluginsModulesPath,
state,
treeShakeNext,
}),
),
)
// TODO: switch to Array.flat() once we drop support for Node.js < 11.0.0
// eslint-disable-next-line unicorn/prefer-spread
Expand All @@ -82,20 +103,22 @@ const getFileDependencies = async function ({ path, packageJson, pluginsModulesP
const getImportDependencies = function ({
dependency,
basedir,
featureFlags,
packageJson,
pluginsModulesPath,
state,
treeShakeNext,
pluginsModulesPath,
}) {
const shouldTreeShakeNext = treeShakeNext || isNextOnNetlify(dependency)
if (shouldTreeShake(dependency, shouldTreeShakeNext)) {
return getTreeShakedDependencies({
dependency,
basedir,
featureFlags,
packageJson,
pluginsModulesPath,
state,
treeShakeNext: shouldTreeShakeNext,
pluginsModulesPath,
})
}

Expand All @@ -110,13 +133,21 @@ const isNextOnNetlify = function (dependency) {
const getTreeShakedDependencies = async function ({
dependency,
basedir,
featureFlags,
packageJson,
pluginsModulesPath,
state,
treeShakeNext,
pluginsModulesPath,
}) {
const path = await resolvePathPreserveSymlinks(dependency, [basedir, pluginsModulesPath].filter(Boolean))
const depsPath = await getFileDependencies({ path, packageJson, state, treeShakeNext, pluginsModulesPath })
const depsPath = await getFileDependencies({
featureFlags,
path,
packageJson,
pluginsModulesPath,
state,
treeShakeNext,
})
return [path, ...depsPath]
}

Expand Down
3 changes: 2 additions & 1 deletion src/runtimes/node/index.js
Expand Up @@ -42,14 +42,14 @@ const zipFunction = async function ({
config = {},
destFolder,
extension,
featureFlags,
filename,
mainFile,
name,
pluginsModulesPath,
srcDir,
srcPath,
stat,
featureFlags,
}) {
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.
Expand All @@ -67,6 +67,7 @@ const zipFunction = async function ({
config,
destFolder,
extension,
featureFlags,
filename,
mainFile,
pluginsModulesPath,
Expand Down
51 changes: 51 additions & 0 deletions src/runtimes/node/list_imports.js
@@ -0,0 +1,51 @@
const esbuild = require('@netlify/esbuild')
const isBuiltinModule = require('is-builtin-module')
const { tmpName } = require('tmp-promise')

const { safeUnlink } = require('../../utils/fs')

const getListImportsPlugin = ({ imports, path }) => ({
name: 'list-imports',
setup(build) {
build.onResolve({ filter: /.*/ }, (args) => {
const isEntryPoint = args.path === path
const isImport = !isEntryPoint && !isBuiltinModule(args.path)

if (isImport) {
imports.add(args.path)
}

return {
namespace: 'list-imports',
external: isImport,
}
})
},
})

const listImports = async ({ path }) => {
// We're not interested in the output that esbuild generates, we're just
// using it for its parsing capabilities in order to find import/require
// statements. However, if we don't give esbuild a path in `outfile`, it
// will pipe the output to stdout, which we also don't want. So we create
// a temporary file to serve as the esbuild output and then get rid of it
// when we're done.
const targetPath = await tmpName()
const imports = new Set()

try {
await esbuild.build({
entryPoints: [path],
bundle: true,
outfile: targetPath,
platform: 'node',
plugins: [getListImportsPlugin({ imports, path })],
})
} finally {
await safeUnlink(targetPath)
}

return [...imports]
}

module.exports = { listImports }
10 changes: 9 additions & 1 deletion src/runtimes/node/src_files.js
Expand Up @@ -53,6 +53,7 @@ const getSrcFiles = async function ({ config, ...parameters }) {
const getSrcFilesAndExternalModules = async function ({
bundler,
externalNodeModules = [],
featureFlags,
includedFiles = [],
includedFilesBasePath,
mainFile,
Expand All @@ -64,7 +65,14 @@ const getSrcFilesAndExternalModules = async function ({
const includedFilePaths = await getPathsOfIncludedFiles(includedFiles, includedFilesBasePath)

if (bundler === JS_BUNDLER_ZISI) {
const paths = await listFilesUsingLegacyBundler({ srcPath, mainFile, srcDir, stat, pluginsModulesPath })
const paths = await listFilesUsingLegacyBundler({
featureFlags,
srcPath,
mainFile,
srcDir,
stat,
pluginsModulesPath,
})

return {
moduleNames: [],
Expand Down
2 changes: 2 additions & 0 deletions src/runtimes/node/zip_zisi.js
Expand Up @@ -12,6 +12,7 @@ const zipZisi = async ({
config,
destFolder,
extension,
featureFlags,
filename,
mainFile,
pluginsModulesPath,
Expand All @@ -22,6 +23,7 @@ const zipZisi = async ({
const { paths: srcFiles } = await getSrcFilesAndExternalModules({
bundler: JS_BUNDLER_ZISI,
extension,
featureFlags,
includedFiles: config.includedFiles,
includedFilesBasePath: config.includedFilesBasePath || basePath,
mainFile,
Expand Down
4 changes: 4 additions & 0 deletions tests/fixtures/local-node-module-deep-require/function.js
@@ -0,0 +1,4 @@
const mock = require('@netlify/mock-package')
const stack = require('@netlify/mock-package/stack')

module.exports = { mock, stack }

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

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

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

7 changes: 7 additions & 0 deletions tests/fixtures/local-node-module-deep-require/package.json
@@ -0,0 +1,7 @@
{
"name": "local-node-module",
"version": "1.0.0",
"dependencies": {
"@netlify/mock-package": "1.0.0"
}
}
6 changes: 3 additions & 3 deletions tests/helpers/main.js
Expand Up @@ -3,10 +3,10 @@ const { env } = require('process')
const { promisify } = require('util')

const AdmZip = require('adm-zip')
const precinct = require('precinct')
const { dir: getTmpDir } = require('tmp-promise')

const { zipFunctions } = require('../..')
const { listImports } = require('../../src/runtimes/node/list_imports')
const { ARCHIVE_FORMAT_ZIP } = require('../../src/utils/consts')

const FIXTURES_DIR = join(__dirname, '..', 'fixtures')
Expand Down Expand Up @@ -77,8 +77,8 @@ const replaceUnzipPath = function ({ path }) {
// Returns a list of paths included using `require` calls. Relative requires
// will be traversed recursively up to a depth defined by `depth`. All the
// required paths — relative or not — will be returned in a flattened array.
const getRequires = function ({ depth = Number.POSITIVE_INFINITY, filePath }, currentDepth = 1) {
const requires = precinct.paperwork(filePath, { includeCore: false })
const getRequires = async function ({ depth = Number.POSITIVE_INFINITY, filePath }, currentDepth = 1) {
const requires = await listImports({ path: filePath })

if (currentDepth >= depth) {
return requires
Expand Down

1 comment on commit 5bf51d0

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

⏱ Benchmark results

largeDepsEsbuild: 12.2s

largeDepsZisi: 1m 5.8s

Please sign in to comment.