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

Commit

Permalink
fix: allow Node modules to be excluded from the bundle (#673)
Browse files Browse the repository at this point in the history
* fix: allow Node modules to be excluded from the bundle

* chore: fix test

* chore: add build step to test:dev script
  • Loading branch information
eduardoboucas committed Oct 6, 2021
1 parent 1e3ac5a commit 5078244
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 20 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"format:check-fix:prettier": "run-e format:check:prettier format:fix:prettier",
"format:check:prettier": "cross-env-shell prettier --check $npm_package_config_prettier",
"format:fix:prettier": "cross-env-shell prettier --write $npm_package_config_prettier",
"test:dev": "ava",
"test:dev": "npm run build && ava",
"test:ci": "npm run build && nyc -r lcovonly -r text -r json ava"
},
"config": {
Expand Down
7 changes: 7 additions & 0 deletions src/node_dependencies/traverse.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ const getDependencyNamesAndPathsForDependencies = async function ({
state = getNewCache(),
pluginsModulesPath,
}) {
if (dependencyNames.length === 0) {
return {
moduleNames: [],
paths: [],
}
}

const packageJson = await getPackageJson(basedir)
const dependencies = await Promise.all(
dependencyNames.map((dependencyName) =>
Expand Down
51 changes: 32 additions & 19 deletions src/runtimes/node/src_files.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
const { normalize } = require('path')
const { normalize, resolve } = require('path')
const { promisify } = require('util')

const glob = require('glob')
const minimatch = require('minimatch')

const pGlob = promisify(glob)

const { getDependencyNamesAndPathsForDependencies, listFilesUsingLegacyBundler } = require('../../node_dependencies')
const { JS_BUNDLER_ZISI } = require('../../utils/consts')

// Returns the subset of `paths` that don't match any of the glob expressions
// from `exclude`.
const filterExcludedPaths = (paths, exclude = []) => {
if (exclude.length === 0) {
return paths
}

const excludedPaths = paths.filter((path) => !exclude.some((pattern) => minimatch(path, pattern)))

return excludedPaths
}

const getPathsOfIncludedFiles = async (includedFiles, basePath) => {
// Some of the globs in `includedFiles` might be exclusion patterns, which
// means paths that should NOT be included in the bundle. We need to treat
Expand All @@ -16,9 +29,11 @@ const getPathsOfIncludedFiles = async (includedFiles, basePath) => {
const { include, exclude } = includedFiles.reduce(
(acc, path) => {
if (path.startsWith('!')) {
const excludePath = resolve(basePath, path.slice(1))

return {
...acc,
exclude: [...acc.exclude, path.slice(1)],
exclude: [...acc.exclude, excludePath],
}
}

Expand All @@ -38,7 +53,7 @@ const getPathsOfIncludedFiles = async (includedFiles, basePath) => {
const paths = pathGroups.flat()
const normalizedPaths = paths.map(normalize)

return [...new Set(normalizedPaths)]
return { exclude, paths: [...new Set(normalizedPaths)] }
}

const getSrcFiles = async function ({ config, ...parameters }) {
Expand All @@ -63,10 +78,13 @@ const getSrcFilesAndExternalModules = async function ({
srcPath,
stat,
}) {
const includedFilePaths = await getPathsOfIncludedFiles(includedFiles, includedFilesBasePath)
const { exclude: excludedPaths, paths: includedFilePaths } = await getPathsOfIncludedFiles(
includedFiles,
includedFilesBasePath,
)

if (bundler === JS_BUNDLER_ZISI) {
const paths = await listFilesUsingLegacyBundler({
const dependencyPaths = await listFilesUsingLegacyBundler({
featureFlags,
srcPath,
mainFile,
Expand All @@ -75,27 +93,22 @@ const getSrcFilesAndExternalModules = async function ({
stat,
pluginsModulesPath,
})
const includedPaths = filterExcludedPaths([...dependencyPaths, ...includedFilePaths], excludedPaths)

return {
moduleNames: [],
paths: [...paths, ...includedFilePaths],
paths: includedPaths,
}
}

if (externalNodeModules.length !== 0) {
const { moduleNames, paths } = await getDependencyNamesAndPathsForDependencies({
dependencies: externalNodeModules,
basedir: srcDir,
pluginsModulesPath,
})

return { moduleNames, paths: [...paths, ...includedFilePaths, mainFile] }
}
const { moduleNames, paths: dependencyPaths } = await getDependencyNamesAndPathsForDependencies({
dependencies: externalNodeModules,
basedir: srcDir,
pluginsModulesPath,
})
const includedPaths = filterExcludedPaths([...dependencyPaths, ...includedFilePaths], excludedPaths)

return {
moduleNames: externalNodeModules,
paths: [mainFile, ...includedFilePaths],
}
return { moduleNames, paths: [...includedPaths, mainFile] }
}

module.exports = { getSrcFiles, getSrcFilesAndExternalModules }
21 changes: 21 additions & 0 deletions tests/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -1748,6 +1748,27 @@ test('Negated files in `included_files` are excluded from the bundle even if the
t.throws(() => func('en'))
})

testMany(
'Negated files in `included_files` are excluded from the bundle even if they match Node modules required in a function',
['bundler_default', 'bundler_default_parse_esbuild', 'bundler_esbuild'],
async (options, t) => {
const fixtureName = 'node-module-included-try-catch'
const opts = merge(options, {
basePath: join(FIXTURES_DIR, fixtureName),
config: {
'*': {
externalNodeModules: ['test'],
includedFiles: ['!node_modules/test/**'],
},
},
})
const { tmpDir } = await zipNode(t, fixtureName, { opts })

t.true(await pathExists(`${tmpDir}/function.js`))
t.false(await pathExists(`${tmpDir}/node_modules/test/index.js`))
},
)

test('Creates dynamic import shims for functions with the same name and same shim contents with no naming conflicts', async (t) => {
const FUNCTION_COUNT = 30
const fixtureName = 'node-module-dynamic-import-3'
Expand Down

1 comment on commit 5078244

@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.9s

largeDepsZisi: 1m 10.2s

Please sign in to comment.