diff --git a/package-lock.json b/package-lock.json index 1e70fc614..d490cb612 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,6 +23,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", @@ -2077,7 +2078,6 @@ "version": "3.2.0", "resolved": "https://registry.npmjs.org/builtin-modules/-/builtin-modules-3.2.0.tgz", "integrity": "sha512-lGzLKcioL90C7wMczpkY0n/oART3MbBa8R9OFGE1rJxoVI86u4WAGfEk8Wjv10eKSyTHVGkSo3bvBylCEtk7LA==", - "dev": true, "engines": { "node": ">=6" }, @@ -6311,7 +6311,6 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/is-builtin-module/-/is-builtin-module-3.1.0.tgz", "integrity": "sha512-OV7JjAgOTfAFJmHZLvpSTb4qi0nIILDV1gWPYDnDJUTNFM5aGlRAhk4QcT8i7TuAleeEV5Fdkqn3t4mS+Q11fg==", - "dev": true, "dependencies": { "builtin-modules": "^3.0.0" }, @@ -13383,8 +13382,7 @@ "builtin-modules": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/builtin-modules/-/builtin-modules-3.2.0.tgz", - "integrity": "sha512-lGzLKcioL90C7wMczpkY0n/oART3MbBa8R9OFGE1rJxoVI86u4WAGfEk8Wjv10eKSyTHVGkSo3bvBylCEtk7LA==", - "dev": true + "integrity": "sha512-lGzLKcioL90C7wMczpkY0n/oART3MbBa8R9OFGE1rJxoVI86u4WAGfEk8Wjv10eKSyTHVGkSo3bvBylCEtk7LA==" }, "cache-base": { "version": "1.0.1", @@ -16549,7 +16547,6 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/is-builtin-module/-/is-builtin-module-3.1.0.tgz", "integrity": "sha512-OV7JjAgOTfAFJmHZLvpSTb4qi0nIILDV1gWPYDnDJUTNFM5aGlRAhk4QcT8i7TuAleeEV5Fdkqn3t4mS+Q11fg==", - "dev": true, "requires": { "builtin-modules": "^3.0.0" } diff --git a/package.json b/package.json index 3bb31b206..c62297fc0 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/feature_flags.js b/src/feature_flags.js index 5c8600450..651dfba78 100644 --- a/src/feature_flags.js +++ b/src/feature_flags.js @@ -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) => diff --git a/src/main.js b/src/main.js index fc4aba1ba..4b6259416 100644 --- a/src/main.js +++ b/src/main.js @@ -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 @@ -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, @@ -59,6 +60,7 @@ const getListedFunctionFiles = async function ( const getSrcFiles = function ({ bundler, config, + featureFlags, runtime, stat, mainFile, @@ -77,6 +79,7 @@ const getSrcFiles = function ({ bundler, config, extension, + featureFlags, srcPath, mainFile, srcDir, diff --git a/src/node_dependencies/index.js b/src/node_dependencies/index.js index cef1c8a63..51e5a3b21 100644 --- a/src/node_dependencies/index.js +++ b/src/node_dependencies/index.js @@ -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') @@ -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)] @@ -43,19 +52,26 @@ 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 [] } @@ -63,16 +79,21 @@ const getFileDependencies = async function ({ path, packageJson, pluginsModulesP 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 @@ -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, }) } @@ -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] } diff --git a/src/runtimes/node/index.js b/src/runtimes/node/index.js index 7eb80c2c8..64198ced7 100644 --- a/src/runtimes/node/index.js +++ b/src/runtimes/node/index.js @@ -42,6 +42,7 @@ const zipFunction = async function ({ config = {}, destFolder, extension, + featureFlags, filename, mainFile, name, @@ -49,7 +50,6 @@ const zipFunction = async function ({ 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. @@ -67,6 +67,7 @@ const zipFunction = async function ({ config, destFolder, extension, + featureFlags, filename, mainFile, pluginsModulesPath, diff --git a/src/runtimes/node/list_imports.js b/src/runtimes/node/list_imports.js new file mode 100644 index 000000000..8ea4fe05d --- /dev/null +++ b/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 } diff --git a/src/runtimes/node/src_files.js b/src/runtimes/node/src_files.js index bfcd759cf..8df3adf8e 100644 --- a/src/runtimes/node/src_files.js +++ b/src/runtimes/node/src_files.js @@ -53,6 +53,7 @@ const getSrcFiles = async function ({ config, ...parameters }) { const getSrcFilesAndExternalModules = async function ({ bundler, externalNodeModules = [], + featureFlags, includedFiles = [], includedFilesBasePath, mainFile, @@ -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: [], diff --git a/src/runtimes/node/zip_zisi.js b/src/runtimes/node/zip_zisi.js index b38fc291c..3eead747a 100644 --- a/src/runtimes/node/zip_zisi.js +++ b/src/runtimes/node/zip_zisi.js @@ -12,6 +12,7 @@ const zipZisi = async ({ config, destFolder, extension, + featureFlags, filename, mainFile, pluginsModulesPath, @@ -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, diff --git a/tests/fixtures/local-node-module-deep-require/function.js b/tests/fixtures/local-node-module-deep-require/function.js new file mode 100644 index 000000000..ddca32ab2 --- /dev/null +++ b/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 } diff --git a/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/index.js b/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/index.js new file mode 100644 index 000000000..dc03a1aeb --- /dev/null +++ b/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/index.js @@ -0,0 +1,3 @@ +const stack = require('./stack') + +module.exports = { stack } diff --git a/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/package.json b/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/package.json new file mode 100644 index 000000000..970c87ddd --- /dev/null +++ b/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/package.json @@ -0,0 +1,4 @@ +{ + "name": "@netlify/mock-package", + "version": "1.0.0" +} diff --git a/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/stack.js b/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/stack.js new file mode 100644 index 000000000..5c0656140 --- /dev/null +++ b/tests/fixtures/local-node-module-deep-require/node_modules/@netlify/mock-package/stack.js @@ -0,0 +1 @@ +module.exports = 'jam' diff --git a/tests/fixtures/local-node-module-deep-require/package.json b/tests/fixtures/local-node-module-deep-require/package.json new file mode 100644 index 000000000..a848babef --- /dev/null +++ b/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" + } +} diff --git a/tests/helpers/main.js b/tests/helpers/main.js index 342bcf0de..a27e9282c 100644 --- a/tests/helpers/main.js +++ b/tests/helpers/main.js @@ -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') @@ -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 diff --git a/tests/main.js b/tests/main.js index 7026b88e8..57598228d 100644 --- a/tests/main.js +++ b/tests/main.js @@ -135,6 +135,29 @@ testBundlers('Can require node modules', [ESBUILD, ESBUILD_ZISI, DEFAULT], async await zipNode(t, 'local-node-module', { opts: { config: { '*': { nodeBundler: bundler } } } }) }) +testBundlers('Can require deep paths in node modules', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => { + const { tmpDir } = await zipNode(t, 'local-node-module-deep-require', { + opts: { config: { '*': { nodeBundler: bundler } } }, + }) + + // eslint-disable-next-line import/no-dynamic-require, node/global-require + const func = require(`${tmpDir}/function.js`) + + t.deepEqual(func, { mock: { stack: 'jam' }, stack: 'jam' }) + + if (bundler === DEFAULT) { + // TO DO: Remove when `parseWithEsbuild` feature flag is decommissioned. + const { tmpDir: tmpDir2 } = await zipNode(t, 'local-node-module-deep-require', { + opts: { config: { '*': { nodeBundler: bundler } } }, + }) + + // eslint-disable-next-line import/no-dynamic-require, node/global-require + const func2 = require(`${tmpDir2}/function.js`) + + t.deepEqual(func2, { mock: { stack: 'jam' }, stack: 'jam' }) + } +}) + testBundlers('Can require scoped node modules', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => { await zipNode(t, 'node-module-scope', { opts: { config: { '*': { nodeBundler: bundler } } } }) }) @@ -307,6 +330,11 @@ testBundlers( testBundlers('Can require local files deeply', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => { await zipNode(t, 'local-deep-require', { opts: { config: { '*': { nodeBundler: bundler } } } }) + + // TO DO: Remove when `parseWithEsbuild` feature flag is decommissioned. + await zipNode(t, 'local-deep-require', { + opts: { config: { '*': { nodeBundler: bundler } }, featureFlags: { parseWithEsbuild: true } }, + }) }) testBundlers( @@ -432,6 +460,11 @@ testBundlers( testBundlers('Works with many dependencies', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => { await zipNode(t, 'many-dependencies', { opts: { config: { '*': { nodeBundler: bundler } } } }) + + // TO DO: Remove when `parseWithEsbuild` feature flag is decommissioned. + await zipNode(t, 'many-dependencies', { + opts: { config: { '*': { nodeBundler: bundler } }, featureFlags: { parseWithEsbuild: true } }, + }) }) testBundlers('Works with many function files', [ESBUILD, ESBUILD_ZISI, DEFAULT], async (bundler, t) => {