diff --git a/README.md b/README.md index 9ec86af..9512d8c 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,5 @@ # npm-packlist -[![Build Status](https://travis-ci.com/npm/npm-packlist.svg?token=hHeDp9pQmz9kvsgRNVHy&branch=master)](https://travis-ci.com/npm/npm-packlist) - Get a list of the files to add from a folder into an npm package. These can be handed to [tar](http://npm.im/tar) like so to make an npm @@ -29,8 +27,8 @@ This uses the following rules: 1. If a `package.json` file is found, and it has a `files` list, then ignore everything that isn't in `files`. Always include the - readme, license, notice, changes, changelog, and history files, if - they exist, and the package.json file itself. + readme, license, licence and copying files, if they exist, as well + as the package.json file itself. 2. If there's no `package.json` file (or it has no `files` list), and there is a `.npmignore` file, then ignore all the files in the `.npmignore` file. @@ -64,17 +62,14 @@ semantics are applied. ### Interaction between `package.json` and `.npmignore` rules -For simplicity, it is best to use _either_ a `files` list in `package.json` -_or_ a `.npmignore` file, and not both. If you only use one of these -methods, you can skip this documentation section. +In previous versions of this library, the `files` list in `package.json` +was used as an initial filter to drive further tree walking. That is no +longer the case as of version 6.0.0. -The `files` list in `package.json` is used to direct the exploration of the -tree. In other words, that's all the walker will ever look at when -exploring that level. +If you have a `package.json` file with a `files` array within, any top +level `.npmignore` and `.gitignore` files *will be ignored*. -In some cases this can lead to a `.npmignore` file being ignored. If a -_directory_ is listed in `files`, then any rules in a root or nested -`.npmignore` files will be honored. +If a _directory_ is listed in `files`, then any rules in nested `.npmignore` files within that directory will be honored. For example, with this package.json: @@ -88,7 +83,7 @@ a `.npmignore` file at `dir/.npmignore` (and any subsequent sub-directories) will be honored. However, a `.npmignore` at the root level will be skipped. -Conversely, with this package.json: +Additionally, with this package.json: ``` { @@ -96,51 +91,13 @@ Conversely, with this package.json: } ``` -a `.npmignore` file at `dir/.npmignore` will not be honored. - -Any specific file matched by a glob or filename in the package.json `files` -list will be included, and cannot be excluded by any `.npmignore` files in -nested directories, or by a `.npmignore` file in the root package -directory, unless that root `.npmignore` file is also in the `files` list. - -The previous (v1) implementation used in npm 6 and below treated -`package.json` as a special sort of "reverse ignore" file. That is, it was -parsed and handled as if it was a `.npmignore` file with `!` prepended to -all of the globs in the `files` list. In order to include children of a -directory listed in `files`, they would _also_ have `/**` appended to them. - -This is tricky to explain, but is a significant improvement over the -previous (v1) implementation used in npm 6 and below, with the following -beneficial properties: - -- If you have `{"files":["lib"]}` in `package.json`, then the walker will - still ignore files such as `lib/.DS_Store` and `lib/.foo.swp`. The - previous implementation would include these files, as they'd be matched - by the computed `!lib/**` ignore rule. -- If you have `{"files":["lib/a.js","lib/b.js"]}` in `package.json`, and a - `lib/.npmignore` containing `a.js`, then the walker will still include - the two files indicated in `package.json`, and ignore the - `lib/.npmignore` file. The previous implementation would mark these - files for inclusion, but then _exclude_ them when it came to the nested - `.npmignore` file. (Ignore file semantics dictate that a "closer" ignore - file always takes precedence.) -- A file in `lib/pkg-template/package.json` will be included, and its - `files` list will not have any bearing on other files being included or - skipped. When treating `package.json` as just Yet Another ignore file, - this was not the case, leading to difficulty for modules that aim to - initialize a project. - -In general, this walk should work as a reasonable developer would expect. -Matching human expectation is tricky business, and if you find cases where -it violates those expectations, [please let us -know](https://github.com/npm/npm-packlist/issues). +a `.npmignore` file at `dir/.npmignore` will be honored, as well as `dir/subdir/.npmignore`. + +Any specific file matched by an exact filename in the package.json `files` list will be included, and cannot be excluded, by any `.npmignore` files. ## API Same API as [ignore-walk](http://npm.im/ignore-walk), just hard-coded file list and rule sets. -The `Walker` and `WalkerSync` classes take a `bundled` argument, which -is a list of package names to include from node_modules. When calling -the top-level `packlist()` and `packlist.sync()` functions, this -module calls into `npm-bundled` directly. +The `Walker` class will load an [arborist](https://github.com/npm/cli/tree/latest/workspaces/arborist) tree, and if any bundled dependencies are found will include them as well as their own dependencies in the resulting file set. diff --git a/lib/index.js b/lib/index.js index bd72329..bcd0a9b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,87 +1,19 @@ 'use strict' -// Do a two-pass walk, first to get the list of packages that need to be -// bundled, then again to get the actual files and folders. -// Keep a cache of node_modules content and package.json data, so that the -// second walk doesn't have to re-do all the same work. +const Arborist = require('@npmcli/arborist') +const { Walker: IgnoreWalker } = require('ignore-walk') +const { lstatSync: lstat, readFileSync: readFile } = require('fs') +const { basename, dirname, extname, join, relative, resolve, sep } = require('path') -const bundleWalk = require('npm-bundled') -const BundleWalker = bundleWalk.BundleWalker +// symbols used to represent synthetic rule sets +const defaultRules = Symbol('npm-packlist.rules.default') +const strictRules = Symbol('npm-packlist.rules.strict') -const ignoreWalk = require('ignore-walk') -const IgnoreWalker = ignoreWalk.Walker - -const rootBuiltinRules = Symbol('root-builtin-rules') -const packageNecessaryRules = Symbol('package-necessary-rules') -const path = require('path') - -const normalizePackageBin = require('npm-normalize-package-bin') - -// Weird side-effect of this: a readme (etc) file will be included -// if it exists anywhere within a folder with a package.json file. -// The original intent was only to include these files in the root, -// but now users in the wild are dependent on that behavior for -// localized documentation and other use cases. Adding a `/` to -// these rules, while tempting and arguably more "correct", is a -// significant change that will break existing use cases. -const packageMustHaveFileNames = 'readme|copying|license|licence' - -const packageMustHaves = `@(${packageMustHaveFileNames}){,.*[^~$]}` -const packageMustHavesRE = new RegExp(`^(${packageMustHaveFileNames})(\\..*[^~$])?$`, 'i') - -const fs = require('fs') -const glob = require('glob') -const globify = pattern => pattern.split('\\').join('/') - -const readOutOfTreeIgnoreFiles = (root, rel, result = '') => { - for (const file of ['.npmignore', '.gitignore']) { - try { - const ignoreContent = fs.readFileSync(path.join(root, file), { encoding: 'utf8' }) - result += ignoreContent + '\n' - // break the loop immediately after concatting, this allows us to prioritize the - // .npmignore and discard the .gitignore if one exists - break - } catch (err) { - // we ignore ENOENT errors completely because we don't care if the file doesn't exist - // but we throw everything else because failing to read a file that does exist is - // something that the user likely wants to know about. we don't need to test this. - /* istanbul ignore next */ - if (err.code !== 'ENOENT') { - throw err - } - } - } - - if (!rel) { - return result - } - - const firstRel = rel.split(path.sep)[0] - const newRoot = path.join(root, firstRel) - const newRel = path.relative(newRoot, path.join(root, rel)) - - return readOutOfTreeIgnoreFiles(newRoot, newRel, result) -} - -const pathHasPkg = (input) => { - if (!input.startsWith('node_modules/')) { - return false - } - - const segments = input.slice('node_modules/'.length).split('/', 2) - return segments[0].startsWith('@') - ? segments.length === 2 - : true -} - -const pkgFromPath = (input) => { - const segments = input.slice('node_modules/'.length).split('/', 2) - return segments[0].startsWith('@') - ? segments.join('/') - : segments[0] -} +// There may be others, but :?|<> are handled by node-tar +const nameIsBadForWindows = file => /\*/.test(file) -const defaultRules = [ +// these are the default rules that are applied to everything except for non-link bundled deps +const defaults = [ '.npmignore', '.gitignore', '**/.git', @@ -103,410 +35,425 @@ const defaultRules = [ '._*', '**/._*/**', '*.orig', - '/package-lock.json', - '/yarn.lock', - '/pnpm-lock.yaml', '/archived-packages/**', ] -// There may be others, but :?|<> are handled by node-tar -const nameIsBadForWindows = file => /\*/.test(file) +const strictDefaults = [ + // these are forcibly included at all levels + '!/readme{,.*[^~$]}', + '!/copying{,.*[^~$]}', + '!/license{,.*[^~$]}', + '!/licence{,.*[^~$]}', + // these are forcibly excluded + '/.git', +] -class Walker extends IgnoreWalker { - constructor (opt) { - opt = opt || {} +const normalizePath = (path) => path.split('\\').join('/') - // the order in which rules are applied. - opt.ignoreFiles = [ - rootBuiltinRules, - 'package.json', - '.npmignore', - '.gitignore', - packageNecessaryRules, - ] - - opt.includeEmpty = false - opt.path = opt.path || process.cwd() - - // only follow links in the root node_modules folder, because if those - // folders are included, it's because they're bundled, and bundles - // should include the contents, not the symlinks themselves. - // This regexp tests to see that we're either a node_modules folder, - // or a @scope within a node_modules folder, in the root's node_modules - // hierarchy (ie, not in test/foo/node_modules/ or something). - const followRe = /^(?:\/node_modules\/(?:@[^/]+\/[^/]+|[^/]+)\/)*\/node_modules(?:\/@[^/]+)?$/ - const rootPath = opt.parent ? opt.parent.root : opt.path - const followTestPath = opt.path.replace(/\\/g, '/').slice(rootPath.length) - opt.follow = followRe.test(followTestPath) - - super(opt) - - // ignore a bunch of things by default at the root level. - // also ignore anything in the main project node_modules hierarchy, - // except bundled dependencies - if (this.isProject) { - this.bundled = opt.bundled || [] - this.bundledScopes = Array.from(new Set( - this.bundled.filter(f => /^@/.test(f)) - .map(f => f.split('/')[0]))) - this.packageJsonCache = this.parent ? this.parent.packageJsonCache - : (opt.packageJsonCache || new Map()) - let rules = defaultRules.join('\n') + '\n' - - if (opt.prefix && opt.workspaces) { - const gPath = globify(opt.path) - const gPrefix = globify(opt.prefix) - const gWorkspaces = opt.workspaces.map((ws) => globify(ws)) - // if opt.path and opt.prefix are not the same directory, and opt.workspaces has opt.path - // in it, then we know that opt.path is a workspace directory. in order to not drop ignore - // rules from directories between the workspace root (opt.prefix) and the workspace itself - // (opt.path), we need to find and read those now - /* istanbul ignore else */ - if (gPath !== gPrefix && gWorkspaces.includes(gPath)) { - // relpath is the relative path between the prefix and the parent of opt.path - // we use the parent because ignore-walk will read the files in opt.path already - const relpath = path.relative(opt.prefix, path.dirname(opt.path)) - rules += readOutOfTreeIgnoreFiles(opt.prefix, relpath) - } else if (gPath === gPrefix) { - // on the other hand, if the path and the prefix are the same, then we ignore workspaces - // so that we don't pack workspaces inside of a root project - rules += opt.workspaces.map((ws) => globify(path.relative(opt.path, ws))).join('\n') - } +const readOutOfTreeIgnoreFiles = (root, rel, result = []) => { + for (const file of ['.npmignore', '.gitignore']) { + try { + const ignoreContent = readFile(join(root, file), { encoding: 'utf8' }) + result.push(ignoreContent) + // break the loop immediately after reading, this allows us to prioritize + // the .npmignore and discard the .gitignore if one is present + break + } catch (err) { + // we ignore ENOENT errors completely because we don't care if the file doesn't exist + // but we throw everything else because failing to read a file that does exist is + // something that the user likely wants to know about + // istanbul ignore next -- we do not need to test a thrown error + if (err.code !== 'ENOENT') { + throw err } - - super.onReadIgnoreFile(rootBuiltinRules, rules, _ => _) - } else { - this.bundled = [] - this.bundledScopes = [] - this.packageJsonCache = this.parent.packageJsonCache } } - get isProject () { - return !this.parent || this.parent.follow && this.isSymbolicLink + if (!rel) { + return result } - onReaddir (entries) { - if (this.isProject) { - entries = entries.filter(e => - e !== '.git' && - !(e === 'node_modules' && this.bundled.length === 0) - ) + const firstRel = rel.split(sep, 1)[0] + const newRoot = join(root, firstRel) + const newRel = relative(newRoot, join(root, rel)) + + return readOutOfTreeIgnoreFiles(newRoot, newRel, result) +} + +class PackWalker extends IgnoreWalker { + constructor (opts) { + const options = { + ...opts, + includeEmpty: false, + follow: false, } - // if we have a package.json, then look in it for 'files' - // we _only_ do this in the root project, not bundled deps - // or other random folders. Bundled deps are always assumed - // to be in the state the user wants to include them, and - // a package.json somewhere else might be a template or - // test or something else entirely. - if (!this.isProject || !entries.includes('package.json')) { - return super.onReaddir(entries) + // we path.resolve() here because ignore-walk doesn't do it and we want full paths + options.path = resolve(options.path || process.cwd()).replace(/\\/g, '/') + + if (!options.ignoreFiles) { + options.ignoreFiles = [ + defaultRules, + 'package.json', + '.npmignore', + '.gitignore', + strictRules, + ] } - // when the cache has been seeded with the root manifest, - // we must respect that (it may differ from the filesystem) - const ig = path.resolve(this.path, 'package.json') + super(options) + this.isPackage = options.isPackage + this.seen = options.seen || new Set() + this.tree = options.tree // no default, we'll load the tree later if we need to + this.requiredFiles = options.requiredFiles || [] + + const additionalDefaults = [] + if (options.prefix && options.workspaces) { + const path = normalizePath(options.path) + const prefix = normalizePath(options.prefix) + const workspaces = options.workspaces.map((ws) => normalizePath(ws)) + + // istanbul ignore else - this does nothing unless we need it to + if (path !== prefix && workspaces.includes(path)) { + // if path and prefix are not the same directory, and workspaces has path in it + // then we know path is a workspace directory. in order to not drop ignore rules + // from directories between the workspaces root (prefix) and the workspace itself + // (path) we need to find and read those now + const relpath = relative(options.prefix, dirname(options.path)) + additionalDefaults.push(...readOutOfTreeIgnoreFiles(options.prefix, relpath)) + } else if (path === prefix) { + // on the other hand, if the path and prefix are the same, then we ignore workspaces + // so that we don't pack a workspace as part of the root project. append them as + // normalized relative paths from the root + additionalDefaults.push(...workspaces.map((w) => normalizePath(relative(options.path, w)))) + } + } - if (this.packageJsonCache.has(ig)) { - const pkg = this.packageJsonCache.get(ig) + // go ahead and inject the default rules now + this.injectRules(defaultRules, [...defaults, ...additionalDefaults]) - // fall back to filesystem when seeded manifest is invalid - if (!pkg || typeof pkg !== 'object') { - return this.readPackageJson(entries) - } + if (!this.isPackage) { + // if this instance is not a package, then place some strict default rules, and append + // known required files for this directory + this.injectRules(strictRules, [ + ...strictDefaults, + ...this.requiredFiles.map((file) => `!${file}`), + ]) + } + } - // feels wonky, but this ensures package bin is _always_ - // normalized, as well as guarding against invalid JSON - return this.getPackageFiles(entries, JSON.stringify(pkg)) + // overridden method: we intercept the reading of the package.json file here so that we can + // process it into both the package.json file rules as well as the strictRules synthetic rule set + addIgnoreFile (file, callback) { + // if we're adding anything other than package.json, then let ignore-walk handle it + if (file !== 'package.json' || !this.isPackage) { + return super.addIgnoreFile(file, callback) } - this.readPackageJson(entries) + return this.processPackage(callback) } - onReadPackageJson (entries, er, pkg) { - if (er) { - this.emit('error', er) - } else { - this.getPackageFiles(entries, pkg) + // overridden method: if we're done, but we're a package, then we also need to evaluate bundles + // before we actually emit our done event + emit (ev, data) { + if (ev !== 'done' || !this.isPackage) { + return super.emit(ev, data) } + + // we intentionally delay the done event while keeping the function sync here + // eslint-disable-next-line promise/catch-or-return, promise/always-return + this.gatherBundles().then(() => { + super.emit('done', this.result) + }) + return true } - mustHaveFilesFromPackage (pkg) { - const files = [] - if (pkg.browser) { - files.push('/' + pkg.browser) - } - if (pkg.main) { - files.push('/' + pkg.main) - } - if (pkg.bin) { - // always an object because normalized already - for (const key in pkg.bin) { - files.push('/' + pkg.bin[key]) - } + // overridden method: before actually filtering, we make sure that we've removed the rules for + // files that should no longer take effect due to our order of precedence + filterEntries () { + if (this.ignoreRules['package.json']) { + // package.json means no .npmignore or .gitignore + this.ignoreRules['.npmignore'] = null + this.ignoreRules['.gitignore'] = null + } else if (this.ignoreRules['.npmignore']) { + // .npmignore means no .gitignore + this.ignoreRules['.gitignore'] = null } - files.push( - '/package.json', - '/npm-shrinkwrap.json', - '!/package-lock.json', - packageMustHaves - ) - return files + + return super.filterEntries() } - getPackageFiles (entries, pkg) { - try { - // XXX this could be changed to use read-package-json-fast - // which handles the normalizing of bins for us, and simplifies - // the test for bundleDependencies and bundledDependencies later. - // HOWEVER if we do this, we need to be sure that we're careful - // about what we write back out since rpj-fast removes some fields - // that the user likely wants to keep. it also would add a second - // file read that we would want to optimize away. - pkg = normalizePackageBin(JSON.parse(pkg.toString())) - } catch (er) { - // not actually a valid package.json - return super.onReaddir(entries) + // overridden method: we never want to include anything that isn't a file or directory + onstat (opts, callback) { + if (!opts.st.isFile() && !opts.st.isDirectory()) { + return callback() } - const ig = path.resolve(this.path, 'package.json') - this.packageJsonCache.set(ig, pkg) + return super.onstat(opts, callback) + } - // no files list, just return the normal readdir() result - if (!Array.isArray(pkg.files)) { - return super.onReaddir(entries) + // overridden method: we want to refuse to pack files that are invalid, node-tar protects us from + // a lot of them but not all + stat (opts, callback) { + if (nameIsBadForWindows(opts.entry)) { + return callback() } - pkg.files.push(...this.mustHaveFilesFromPackage(pkg)) + return super.stat(opts, callback) + } - // If the package has a files list, then it's unlikely to include - // node_modules, because why would you do that? but since we use - // the files list as the effective readdir result, that means it - // looks like we don't have a node_modules folder at all unless we - // include it here. - if ((pkg.bundleDependencies || pkg.bundledDependencies) && entries.includes('node_modules')) { - pkg.files.push('node_modules') + // overridden method: we need to load the arborist tree before we actually start running + start () { + if (this.isPackage && !this.tree) { + const arborist = new Arborist({ path: this.path }) + // loading the tree is async while the start function depends on being sync + // eslint-disable-next-line promise/catch-or-return, promise/always-return + arborist.loadActual().then((tree) => { + this.tree = tree + super.start() + }) + return this } - const patterns = Array.from(new Set(pkg.files)).reduce((set, pattern) => { - const excl = pattern.match(/^!+/) - if (excl) { - pattern = pattern.slice(excl[0].length) - } - // strip off any / or ./ from the start of the pattern. /foo => foo, ./foo => foo - pattern = pattern.replace(/^\.?\/+/, '') - // an odd number of ! means a negated pattern. !!foo ==> foo - const negate = excl && excl[0].length % 2 === 1 - set.push({ pattern, negate }) - return set - }, []) - - let n = patterns.length - const set = new Set() - const negates = new Set() - const results = [] - const then = (pattern, negate, er, fileList, i) => { - if (er) { - return this.emit('error', er) - } + return super.start() + } + + // overridden method: this is called to create options for a child walker when we step + // in to a normal child directory (this will never be a bundle). the default method here + // copies the root's `ignoreFiles` value, but we don't want to respect package.json for + // subdirectories, so we override it with a list that intentionally omits package.json + walkerOpt (entry, opts) { + let ignoreFiles = [ + defaultRules, + '.npmignore', + '.gitignore', + strictRules, + ] - results[i] = { negate, fileList } - if (--n === 0) { - processResults(results) + // however, if we have a tree, and we have workspaces, and the directory we're about + // to step into is a workspace, then we _do_ want to respect its package.json + if (this.tree && this.tree.workspaces) { + const workspaceDirs = [...this.tree.workspaces.values()] + .map((dir) => dir.replace(/\\/g, '/')) + + const entryPath = join(this.path, entry).replace(/\\/g, '/') + if (workspaceDirs.includes(entryPath)) { + ignoreFiles = [ + defaultRules, + 'package.json', + '.npmignore', + '.gitignore', + strictRules, + ] } } - const processResults = processed => { - for (const { negate, fileList } of processed) { - if (negate) { - fileList.forEach(f => { - f = f.replace(/\/+$/, '') - set.delete(f) - negates.add(f) - }) - } else { - fileList.forEach(f => { - f = f.replace(/\/+$/, '') - set.add(f) - negates.delete(f) - }) - } - } - const list = Array.from(set) - // replace the files array with our computed explicit set - pkg.files = list.concat(Array.from(negates).map(f => '!' + f)) - const rdResult = Array.from(new Set( - list.map(f => f.replace(/^\/+/, '')) - )) - super.onReaddir(rdResult) + return { + ...super.walkerOpt(entry, opts), + ignoreFiles, + // we map over our own requiredFiles and pass ones that are within this entry + requiredFiles: this.requiredFiles + .map((file) => { + if (relative(file, entry) === '..') { + return relative(entry, file).replace(/\\/g, '/') + } + return false + }) + .filter(Boolean), } + } - // maintain the index so that we process them in-order only once all - // are completed, otherwise the parallelism messes things up, since a - // glob like **/*.js will always be slower than a subsequent !foo.js - patterns.forEach(({ pattern, negate }, i) => - this.globFiles(pattern, (er, res) => then(pattern, negate, er, res, i))) + // overridden method: we want child walkers to be instances of this class, not ignore-walk + walker (entry, opts, callback) { + new PackWalker(this.walkerOpt(entry, opts)).on('done', callback).start() } - filterEntry (entry, partial) { - // get the partial path from the root of the walk - const p = this.path.slice(this.root.length + 1) - const { isProject } = this - const pkg = isProject && pathHasPkg(entry) - ? pkgFromPath(entry) - : null - const rootNM = isProject && entry === 'node_modules' - const rootPJ = isProject && entry === 'package.json' - - return ( - // if we're in a bundled package, check with the parent. - /^node_modules($|\/)/i.test(p) && !this.isProject ? this.parent.filterEntry( - this.basename + '/' + entry, partial) - - // if package is bundled, all files included - // also include @scope dirs for bundled scoped deps - // they'll be ignored if no files end up in them. - // However, this only matters if we're in the root. - // node_modules folders elsewhere, like lib/node_modules, - // should be included normally unless ignored. - : pkg ? this.bundled.indexOf(pkg) !== -1 || - this.bundledScopes.indexOf(pkg) !== -1 - - // only walk top node_modules if we want to bundle something - : rootNM ? !!this.bundled.length - - // always include package.json at the root. - : rootPJ ? true - - // always include readmes etc in any included dir - : packageMustHavesRE.test(entry) ? true - - // npm-shrinkwrap and package.json always included in the root pkg - : isProject && (entry === 'npm-shrinkwrap.json' || entry === 'package.json') - ? true - - // package-lock never included - : isProject && entry === 'package-lock.json' ? false - - // otherwise, follow ignore-walk's logic - : super.filterEntry(entry, partial) - ) + // overridden method: we use a custom sort method to help compressibility + sort (a, b) { + // optimize for compressibility + // extname, then basename, then locale alphabetically + // https://twitter.com/isntitvacant/status/1131094910923231232 + const exta = extname(a).toLowerCase() + const extb = extname(b).toLowerCase() + const basea = basename(a).toLowerCase() + const baseb = basename(b).toLowerCase() + + return exta.localeCompare(extb, 'en') || + basea.localeCompare(baseb, 'en') || + a.localeCompare(b, 'en') } - filterEntries () { - if (this.ignoreRules['.npmignore']) { - this.ignoreRules['.gitignore'] = null - } - this.filterEntries = super.filterEntries - super.filterEntries() + // convenience method: this joins the given rules with newlines, appends a trailing newline, + // and calls the internal onReadIgnoreFile method + injectRules (filename, rules, callback = () => {}) { + this.onReadIgnoreFile(filename, `${rules.join('\n')}\n`, callback) } - addIgnoreFile (file, then) { - const ig = path.resolve(this.path, file) - if (file === 'package.json' && !this.isProject) { - then() - } else if (this.packageJsonCache.has(ig)) { - this.onPackageJson(ig, this.packageJsonCache.get(ig), then) - } else { - super.addIgnoreFile(file, then) + // custom method: this is called by addIgnoreFile when we find a package.json, it uses the + // arborist tree to pull both default rules and strict rules for the package + processPackage (callback) { + const { + bin, + browser, + files, + main, + } = this.tree.package + + // rules in these arrays are inverted since they are patterns we want to _not_ ignore + const ignores = [] + const strict = [ + ...strictDefaults, + '!/package.json', + '!/npm-shrinkwrap.json', + '/.git', + '/node_modules', + '/package-lock.json', + '/yarn.lock', + '/pnpm-lock.yaml', + ] + + // if we have a files array in our package, we need to pull rules from it + if (files) { + for (const file of files) { + // invert the rule because these are things we want to include + const inverse = `!${file}` + try { + // if an entry in the files array is a specific file, then we need to include it as a + // strict requirement for this package. if it's a directory or a pattern, it's a default + // pattern instead. this is ugly, but we have to stat to find out if it's a file + const stat = lstat(join(this.path, file.replace(/^!+/, '')).replace(/\\/g, '/')) + // if we have a file and we know that, it's strictly required + if (stat.isFile()) { + strict.unshift(inverse) + this.requiredFiles.push(file.startsWith('/') ? file.slice(1) : file) + } else if (stat.isDirectory()) { + // otherwise, it's a default ignore, and since we got here we know it's not a pattern + // so we include the directory contents + ignores.push(inverse) + ignores.push(`${inverse}/**`) + } + // if the thing exists, but is neither a file or a directory, we don't want it at all + } catch (err) { + // if lstat throws, then we assume we're looking at a pattern and treat it as a default + ignores.push(inverse) + } + } + + // we prepend a '*' to exclude everything, followed by our inverted file rules + // which now mean to include those + this.injectRules('package.json', ['*', ...ignores]) } - } - onPackageJson (ig, pkg, then) { - this.packageJsonCache.set(ig, pkg) + // browser is required + if (browser) { + strict.push(`!/${browser}`) + } - if (Array.isArray(pkg.files)) { - // in this case we already included all the must-haves - super.onReadIgnoreFile('package.json', pkg.files.map( - f => '!' + f - ).join('\n') + '\n', then) - } else { - // if there's a bin, browser or main, make sure we don't ignore it - // also, don't ignore the package.json itself, or any files that - // must be included in the package. - const rules = this.mustHaveFilesFromPackage(pkg).map(f => `!${f}`) - const data = rules.join('\n') + '\n' - super.onReadIgnoreFile(packageNecessaryRules, data, then) + // main is required + if (main) { + strict.push(`!/${main}`) } - } - // override parent stat function to completely skip any filenames - // that will break windows entirely. - // XXX(isaacs) Next major version should make this an error instead. - stat ({ entry, file, dir }, then) { - if (nameIsBadForWindows(entry)) { - then() - } else { - super.stat({ entry, file, dir }, then) + // each bin is required + if (bin) { + for (const key in bin) { + strict.push(`!/${bin[key]}`) + } } + + // and now we add all of the strict rules to our synthetic file + this.injectRules(strictRules, strict, callback) } - // override parent onstat function to nix all symlinks, other than - // those coming out of the followed bundled symlink deps - onstat ({ st, entry, file, dir, isSymbolicLink }, then) { - if (st.isSymbolicLink()) { - then() - } else { - super.onstat({ st, entry, file, dir, isSymbolicLink }, then) + // custom method: after we've finished gathering the files for the root package, we call this + // before emitting the 'done' event in order to gather all of the files for bundled deps + async gatherBundles () { + if (this.tree && this.seen.has(this.tree)) { + return } - } - onReadIgnoreFile (file, data, then) { - if (file === 'package.json') { - try { - const ig = path.resolve(this.path, file) - this.onPackageJson(ig, JSON.parse(data), then) - } catch (er) { - // ignore package.json files that are not json - then() - } + // add this node to our seen tracker + this.seen.add(this.tree) + + // if we're the project root, then we look at our bundleDependencies, otherwise we got here + // because we're a bundled dependency of the root, which means we need to include all prod + // and optional dependencies in the bundle + let toBundle + if (this.tree.isProjectRoot) { + const { bundleDependencies } = this.tree.package + toBundle = bundleDependencies || [] } else { - super.onReadIgnoreFile(file, data, then) + const { dependencies, optionalDependencies } = this.tree.package + toBundle = Object.keys(dependencies || {}).concat(Object.keys(optionalDependencies || {})) } - } - sort (a, b) { - // optimize for compressibility - // extname, then basename, then locale alphabetically - // https://twitter.com/isntitvacant/status/1131094910923231232 - const exta = path.extname(a).toLowerCase() - const extb = path.extname(b).toLowerCase() - const basea = path.basename(a).toLowerCase() - const baseb = path.basename(b).toLowerCase() + for (const dep of toBundle) { + const edge = this.tree.edgesOut.get(dep) + // no edgeOut = missing node, so skip it. we can't pack it if it's not here + // we also refuse to pack peer dependencies and dev dependencies + if (!edge || edge.peer || edge.dev) { + continue + } - return exta.localeCompare(extb, 'en') || - basea.localeCompare(baseb, 'en') || - a.localeCompare(b, 'en') - } + // get a reference to the node we're bundling + const node = this.tree.edgesOut.get(dep).to + // and start building options to be passed to the walker for this package + const walkerOpts = { + // we use node.path for the path because we want the location the node was linked to, + // not where it actually lives on disk + path: node.path, + // but link nodes don't have edgesOut, so we need to pass in the target of the node + // in order to make sure we correctly traverse its dependencies + tree: node.target, + isPackage: true, + ignoreFiles: [], + seen: this.seen, // pass through seen so we can prevent infinite circular loops + } - globFiles (pattern, cb) { - glob(globify(pattern), { dot: true, cwd: this.path, nocase: true }, cb) - } + // if our node is a link, we apply defaultRules. we don't do this for regular bundled + // deps because their .npmignore and .gitignore files are excluded by default and may + // override defaults + if (node.isLink) { + walkerOpts.ignoreFiles.push(defaultRules) + } - readPackageJson (entries) { - fs.readFile(this.path + '/package.json', (er, pkg) => - this.onReadPackageJson(entries, er, pkg)) - } + // _all_ nodes will follow package.json rules from their package root + walkerOpts.ignoreFiles.push('package.json') + + // only link nodes will obey .npmignore or .gitignore + if (node.isLink) { + walkerOpts.ignoreFiles.push('.npmignore') + walkerOpts.ignoreFiles.push('.gitignore') + } - walker (entry, opt, then) { - new Walker(this.walkerOpt(entry, opt)).on('done', then).start() + // _all_ nodes follow strict rules + walkerOpts.ignoreFiles.push(strictRules) + + // create a walker for this dependency and gather its results + const walker = new PackWalker(walkerOpts) + const bundled = await new Promise((pResolve, pReject) => { + walker.on('error', pReject) + walker.on('done', pResolve) + walker.start() + }) + + // now we make sure we have our paths correct from the root, and accumulate everything into + // our own result set to deduplicate + const relativeFrom = relative(this.root, walker.path) + for (const file of bundled) { + this.result.add(join(relativeFrom, file).replace(/\\/g, '/')) + } + } } } const walk = (options, callback) => { - options = options || {} - const p = new Promise((resolve, reject) => { - const bw = new BundleWalker(options) - bw.on('done', bundled => { - options.bundled = bundled - options.packageJsonCache = bw.packageJsonCache - new Walker(options).on('done', resolve).on('error', reject).start() - }) - bw.start() + options = { ...options, isPackage: true } + const p = new Promise((pResolve, pReject) => { + new PackWalker(options).on('done', pResolve).on('error', pReject).start() }) return callback ? p.then(res => callback(null, res), callback) : p } module.exports = walk -walk.Walker = Walker +walk.Walker = PackWalker diff --git a/package.json b/package.json index ca87235..d6c237d 100644 --- a/package.json +++ b/package.json @@ -5,12 +5,10 @@ "directories": { "test": "test" }, - "main": "lib", + "main": "lib/index.js", "dependencies": { - "glob": "^8.0.1", - "ignore-walk": "^5.0.1", - "npm-bundled": "^2.0.0", - "npm-normalize-package-bin": "^2.0.0" + "@npmcli/arborist": "^5.0.4", + "ignore-walk": "^5.0.1" }, "author": "GitHub Inc.", "license": "ISC", diff --git a/test/bundle-missing-dep.js b/test/bundle-missing-dep.js new file mode 100644 index 0000000..7cc9e1e --- /dev/null +++ b/test/bundle-missing-dep.js @@ -0,0 +1,33 @@ +'use strict' + +const t = require('tap') +const packlist = require('../') + +t.test('skips bundling deps with missing edges', async (t) => { + const pkg = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test', + version: '1.0.0', + main: 'index.js', + // named in bundleDependencies, but not actually a dependency + bundleDependencies: ['history'], + }), + 'index.js': '', + node_modules: { + history: { + 'package.json': JSON.stringify({ + name: 'history', + version: '1.0.0', + main: 'index.js', + }), + 'index.js': '', + }, + }, + }) + + const files = await packlist({ path: pkg }) + t.same(files, [ + 'index.js', + 'package.json', + ]) +}) diff --git a/test/bundled-cycle.js b/test/bundled-cycle.js new file mode 100644 index 0000000..f8eb4a9 --- /dev/null +++ b/test/bundled-cycle.js @@ -0,0 +1,53 @@ +'use strict' + +const t = require('tap') +const packlist = require('../') + +t.test('correctly bundles cyclic deps', async (t) => { + const pkg = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + main: 'index.js', + dependencies: { + a: '1.0.0', + }, + bundleDependencies: ['a'], + }), + 'index.js': '', + node_modules: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + main: 'index.js', + dependencies: { + b: '1.0.0', + }, + }), + 'index.js': '', + }, + b: { + 'package.json': JSON.stringify({ + name: 'b', + version: '1.0.0', + main: 'index.js', + dependencies: { + a: '1.0.0', + }, + }), + 'index.js': '', + }, + }, + }) + + const files = await packlist({ path: pkg }) + t.same(files, [ + 'index.js', + 'node_modules/a/index.js', + 'node_modules/b/index.js', + 'node_modules/a/package.json', + 'node_modules/b/package.json', + 'package.json', + ]) +}) diff --git a/test/bundled-file-in-workspace.js b/test/bundled-file-in-workspace.js new file mode 100644 index 0000000..695eb26 --- /dev/null +++ b/test/bundled-file-in-workspace.js @@ -0,0 +1,88 @@ +'use strict' + +const t = require('tap') +const packlist = require('../') + +t.test('correctly filters files from workspace subdirectory', async (t) => { + const pkg = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + files: ['docs/*.txt'], + main: 'index.js', + workspaces: ['./docs'], + }), + 'index.js': '', + docs: { + 'package.json': JSON.stringify({ + name: 'docs', + version: '1.0.0', + main: 'index.js', + files: ['*.txt'], + }), + 'bar.txt': '', + 'foo.txt': '', + 'readme.md': '', + test: { + 'index.js': '', + }, + }, + }) + + const files = await packlist({ path: pkg }) + t.same(files, [ + 'index.js', + 'package.json', + 'docs/readme.md', // readme.md is always included + 'docs/bar.txt', + 'docs/foo.txt', + ]) +}) + +t.test('does not filter based on package.json if subdirectory is not a workspace', async (t) => { + const pkg = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + files: ['docs/*.txt'], + main: 'index.js', + // this test needs a workspace to exist, but that workspace cannot be the one we include + // files from + workspaces: ['./unrelated'], + }), + 'index.js': '', + docs: { + 'package.json': JSON.stringify({ + name: 'docs', + version: '1.0.0', + main: 'index.js', + files: ['bar.txt', 'foo.txt'], + }), + 'bar.txt': '', + 'baz.txt': '', + 'foo.txt': '', + 'readme.md': '', + test: { + 'index.js': '', + }, + }, + unrelated: { + 'package.json': JSON.stringify({ + name: 'unrelated', + version: '1.0.0', + main: 'index.js', + }), + 'index.js': '', + }, + }) + + const files = await packlist({ path: pkg }) + t.same(files, [ + 'index.js', + 'package.json', + 'docs/readme.md', // readme.md is always included + 'docs/bar.txt', + 'docs/baz.txt', // was _not_ filtered + 'docs/foo.txt', + ]) +}) diff --git a/test/bundled-files.js b/test/bundled-files.js index bd80d01..9fb6110 100644 --- a/test/bundled-files.js +++ b/test/bundled-files.js @@ -14,6 +14,9 @@ t.test('includes bundled dependency using bundleDependencies', async (t) => { name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + history: '1.0.0', + }, bundleDependencies: [ 'history', ], @@ -24,7 +27,7 @@ t.test('includes bundled dependency using bundleDependencies', async (t) => { node_modules: { history: { 'package.json': JSON.stringify({ - name: '@npmwombat/history', + name: 'history', version: '1.0.0', main: 'index.js', }), @@ -48,6 +51,9 @@ t.test('includes bundled dependency using bundledDependencies', async (t) => { name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + history: '1.0.0', + }, bundledDependencies: [ 'history', ], @@ -58,7 +64,7 @@ t.test('includes bundled dependency using bundledDependencies', async (t) => { node_modules: { history: { 'package.json': JSON.stringify({ - name: '@npmwombat/history', + name: 'history', version: '1.0.0', main: 'index.js', }), diff --git a/test/bundled-scoped-symlink.js b/test/bundled-scoped-symlink.js index 189ad85..55675e5 100644 --- a/test/bundled-scoped-symlink.js +++ b/test/bundled-scoped-symlink.js @@ -14,6 +14,9 @@ const pkg = t.testdir({ name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + '@npmwombat/history': '1.0.0', + }, bundleDependencies: [ '@npmwombat/history', ], diff --git a/test/bundled-scoped.js b/test/bundled-scoped.js index e3859ea..7942758 100644 --- a/test/bundled-scoped.js +++ b/test/bundled-scoped.js @@ -13,6 +13,9 @@ const pkg = t.testdir({ name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + '@npmwombat/history': '1.0.0', + }, bundleDependencies: [ '@npmwombat/history', ], diff --git a/test/bundled-symlink.js b/test/bundled-symlink.js index 0138efe..fc0cfd4 100644 --- a/test/bundled-symlink.js +++ b/test/bundled-symlink.js @@ -14,6 +14,9 @@ const pkg = t.testdir({ name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + history: '1.0.0', + }, bundleDependencies: [ 'history', ], diff --git a/test/bundled-workspace.js b/test/bundled-workspace.js new file mode 100644 index 0000000..bd2cd5a --- /dev/null +++ b/test/bundled-workspace.js @@ -0,0 +1,54 @@ +'use strict' + +const t = require('tap') +const packlist = require('../') + +t.test('packs workspace dependencies correctly', async (t) => { + const pkg = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.2.3', + main: 'index.js', + files: ['index.js'], + dependencies: { + foo: '1.0.0', + bar: '1.0.0', + }, + bundleDependencies: ['foo', 'bar'], + workspaces: ['./workspaces/*'], + }), + 'index.js': '', + workspaces: { + foo: { + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0', + main: 'index.js', + }), + 'index.js': '', + }, + bar: { + 'package.json': JSON.stringify({ + name: 'bar', + version: '1.0.0', + main: 'index.js', + }), + 'index.js': '', + }, + }, + node_modules: { + foo: t.fixture('symlink', '../workspaces/foo'), + bar: t.fixture('symlink', '../workspaces/bar'), + }, + }) + + const files = await packlist({ path: pkg }) + t.same(files, [ + 'index.js', + 'node_modules/bar/index.js', + 'node_modules/foo/index.js', + 'node_modules/bar/package.json', + 'node_modules/foo/package.json', + 'package.json', + ]) +}) diff --git a/test/bundled.js b/test/bundled.js index 3d874f7..6c78e35 100644 --- a/test/bundled.js +++ b/test/bundled.js @@ -14,6 +14,9 @@ t.test('includes bundled dependency using bundleDependencies', async (t) => { name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + history: '1.0.0', + }, bundleDependencies: [ 'history', ], @@ -23,7 +26,7 @@ t.test('includes bundled dependency using bundleDependencies', async (t) => { node_modules: { history: { 'package.json': JSON.stringify({ - name: '@npmwombat/history', + name: 'history', version: '1.0.0', main: 'index.js', }), @@ -47,6 +50,9 @@ t.test('includes bundled dependency using bundledDependencies', async (t) => { name: 'test-package', version: '3.1.4', main: 'elf.js', + dependencies: { + history: '1.0.0', + }, bundledDependencies: [ 'history', ], @@ -56,7 +62,7 @@ t.test('includes bundled dependency using bundledDependencies', async (t) => { node_modules: { history: { 'package.json': JSON.stringify({ - name: '@npmwombat/history', + name: 'history', version: '1.0.0', main: 'index.js', }), diff --git a/test/cannot-include-non-file-or-directory.js b/test/cannot-include-non-file-or-directory.js new file mode 100644 index 0000000..fbfb071 --- /dev/null +++ b/test/cannot-include-non-file-or-directory.js @@ -0,0 +1,62 @@ +'use strict' + +const fs = require('fs') +const t = require('tap') +const { join } = require('path') + +t.test('cannot include something that exists but is neither a file nor a directory', async (t) => { + const pkg = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + main: 'index.js', + files: ['lib', 'device'], + }), + 'index.js': '', + lib: { + socket: '', + }, + device: '', + }) + + // mock fs.lstat for ignore-walk so it tells us that lib/socket is not a file, dir, or symlink + const ignoreWalk = t.mock('ignore-walk', { + fs: { + ...fs, + lstat: (path, options, callback) => { + if (typeof options === 'function') { + callback = options + options = undefined + } + if (path === join(pkg, 'lib', 'socket').replace(/\\/g, '/')) { + return callback(null, { + isFile: () => false, + isDirectory: () => false, + isSymbolicLink: () => false, + }) + } + return fs.lstat(path, options, callback) + }, + }, + }) + + const packlist = t.mock('../', { + 'ignore-walk': ignoreWalk, + fs: { + ...fs, + lstatSync: (path) => { + if (path === join(pkg, 'device').replace(/\\/g, '/')) { + return { isFile: () => false, isDirectory: () => false } + } + + return fs.lstatSync(path) + }, + }, + }) + + const files = await packlist({ path: pkg }) + t.same(files, [ + 'index.js', + 'package.json', + ]) +}) diff --git a/test/package-json-cache.js b/test/package-json-cache.js deleted file mode 100644 index f73723f..0000000 --- a/test/package-json-cache.js +++ /dev/null @@ -1,77 +0,0 @@ -'use strict' - -const t = require('tap') -const path = require('path') -const packlist = require('../') - -const elfJS = ` -module.exports = elf => - console.log("i'm a elf") -` - -const pkg = t.testdir({ - 'package.json': JSON.stringify({ - name: 'test-package', - version: '3.1.4', - files: [ - 'quendi.js', - ], - }), - 'elf.js': elfJS, - 'quendi.js': elfJS, -}) - -const rootManifestPath = path.resolve(pkg, 'package.json') - -t.test('seeded with root manifest', async (t) => { - const packageJsonCache = new Map() - packageJsonCache.set(rootManifestPath, { - files: [ - 'elf.js', - ], - }) - - const files = await packlist({ path: pkg, packageJsonCache }) - t.same(files, [ - 'elf.js', - 'package.json', - ]) -}) - -t.test('seeded with invalid JSON falls back to filesystem', async (t) => { - const packageJsonCache = new Map() - packageJsonCache.set(rootManifestPath, "c'est ne pas une j'son") - - const files = await packlist({ path: pkg, packageJsonCache }) - t.same(files, [ - 'quendi.js', - 'package.json', - ]) -}) - -t.test('when empty', async t => { - const packageJsonCache = new Map() - - const files = await packlist({ path: pkg, packageJsonCache }) - t.same(files, [ - 'quendi.js', - 'package.json', - ]) -}) - -t.test('when not provided at all', async t => { - // can't use the exported function to provide no cache, have to create a Walker instance - const walker = new packlist.Walker({ path: pkg }) - - const files = await new Promise((resolve, reject) => { - walker - .on('done', resolve) - .on('error', reject) - .start() - }) - - t.same(files, [ - 'quendi.js', - 'package.json', - ]) -}) diff --git a/test/package-json-files-and-containing-dir.js b/test/package-json-files-and-containing-dir.js index 44ba939..60ba99e 100644 --- a/test/package-json-files-and-containing-dir.js +++ b/test/package-json-files-and-containing-dir.js @@ -32,7 +32,6 @@ t.test('package with negated files', async (t) => { 'lib/for.js', 'lib/one.js', 'lib/tre.js', - 'lib/two.js', 'package.json', ]) }) diff --git a/test/package-json-files-including-npmignore.js b/test/package-json-files-including-npmignore.js index c55f780..fc00a67 100644 --- a/test/package-json-files-including-npmignore.js +++ b/test/package-json-files-including-npmignore.js @@ -7,10 +7,11 @@ const pkg = t.testdir({ 'package.json': JSON.stringify({ files: [ 'lib/sub/*.js', - '.npmignore', + 'lib/.npmignore', ], }), lib: { + '.npmignore': 'two.js', sub: { 'one.js': 'one', 'two.js': 'two', @@ -19,13 +20,12 @@ const pkg = t.testdir({ }, '.DS_Store': 'a store of ds', }, - '.npmignore': 'two.js', }) t.test('package with negated files', async (t) => { const files = await packlist({ path: pkg }) t.same(files, [ - '.npmignore', + 'lib/.npmignore', 'lib/sub/for.js', 'lib/sub/one.js', 'lib/sub/tre.js', diff --git a/test/package-json-files-nested-dir-and-nested-ignore.js b/test/package-json-files-nested-dir-and-nested-ignore.js index c6feec9..357b1e5 100644 --- a/test/package-json-files-nested-dir-and-nested-ignore.js +++ b/test/package-json-files-nested-dir-and-nested-ignore.js @@ -29,7 +29,6 @@ t.test('package with negated files', async (t) => { 'lib/dir/for.js', 'lib/dir/one.js', 'lib/dir/tre.js', - 'lib/dir/two.js', 'package.json', ]) }) diff --git a/test/package-json-files-no-dir-nested-npmignore.js b/test/package-json-files-no-dir-nested-npmignore.js index cb56ca9..2dd50a8 100644 --- a/test/package-json-files-no-dir-nested-npmignore.js +++ b/test/package-json-files-no-dir-nested-npmignore.js @@ -23,7 +23,6 @@ t.test('package with negated files', async (t) => { 'lib/for.js', 'lib/one.js', 'lib/tre.js', - 'lib/two.js', 'package.json', ]) }) diff --git a/test/package-json-glob-fails.js b/test/package-json-glob-fails.js deleted file mode 100644 index bdc54a5..0000000 --- a/test/package-json-glob-fails.js +++ /dev/null @@ -1,28 +0,0 @@ -'use strict' - -const t = require('tap') - -// mock glob so it throws an error -const glob = (pattern, opt, cb) => cb(new Error('no glob for you')) -const packlist = t.mock('../', { glob }) - -const pkg = t.testdir({ - 'package.json': JSON.stringify({ - files: [ - 'lib', - '!lib/one', - ], - }), - lib: { - one: 'one', - two: 'two', - tre: 'tre', - for: 'for', - '.npmignore': 'two', - '.DS_Store': 'a store of ds', - }, -}) - -t.test('package with busted glob', async (t) => { - await t.rejects(packlist({ path: pkg }), { message: 'no glob for you' }) -}) diff --git a/test/package-json-nested-readme-include-npmignore.js b/test/package-json-nested-readme-include-npmignore.js index e17b02b..c06ab85 100644 --- a/test/package-json-nested-readme-include-npmignore.js +++ b/test/package-json-nested-readme-include-npmignore.js @@ -9,6 +9,12 @@ const pkg = t.testdir({ files: ['.npmignore', 'lib'], }), lib: { + '.npmignore': ` + * + !*.js + !**/*.js + test + `, a: { b: { c: { @@ -42,12 +48,6 @@ const pkg = t.testdir({ 'a.js': 'one', }, }, - '.npmignore': ` - * - !*.js - !**/*.js - test - `, }) t.test('package with negated files', async (t) => { diff --git a/test/package-json-read-fail.js b/test/package-json-read-fail.js deleted file mode 100644 index 77dd026..0000000 --- a/test/package-json-read-fail.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict' - -const t = require('tap') -const fs = require('fs') - -const pkg = t.testdir({ - 'package.json': 'no json here', - 'index.js': '', -}) - -// mock fs.readFile to fail reading package.json -const packlist = t.mock('../', { - fs: { - ...fs, - readFile: (path, options, callback) => { - // this is _not_ an os specific path - if (path === `${pkg}/package.json`) { - if (typeof options === 'function') { - callback = options - options = undefined - } - return callback(new Error('readFile failed')) - } - return fs.readFile(path, options, callback) - }, - }, -}) - -t.test('read fails on package.json', async (t) => { - await t.rejects(packlist({ path: pkg }), { message: 'readFile failed' }) -}) diff --git a/test/package-json-roots-and-nests.js b/test/package-json-roots-and-nests.js index 3a1550f..c803393 100644 --- a/test/package-json-roots-and-nests.js +++ b/test/package-json-roots-and-nests.js @@ -8,6 +8,10 @@ const pkg = t.testdir({ bin: 'bin.js', main: 'main.js', browser: 'browser.js', + dependencies: { + foo: '1.0.0', + '@foo/bar': '1.0.0', + }, bundleDependencies: [ 'foo', '@foo/bar', diff --git a/test/scoped.js b/test/scoped.js index 2da1f89..efce01d 100644 --- a/test/scoped.js +++ b/test/scoped.js @@ -13,6 +13,9 @@ const pkg = t.testdir({ name: 'test-package-scoped', version: '3.1.4', main: 'elf.js', + dependencies: { + '@npmwombat/scoped': '1.0.0', + }, bundledDependencies: [ '@npmwombat/scoped', ], diff --git a/test/workspace.js b/test/workspace.js index ef59058..ed0f449 100644 --- a/test/workspace.js +++ b/test/workspace.js @@ -45,7 +45,8 @@ t.test('respects workspace root ignore files', async (t) => { 'package.json', ]) - // here we leave off workspaces to satisfy coverage + // leave off workspaces to prove that when prefix and root differ there is no change + // in behavior without also specifying workspaces const secondFiles = await packlist({ path: workspacePath, prefix: root, @@ -87,7 +88,7 @@ t.test('packing a workspace root does not include children', async (t) => { }) const workspacePath = path.join(root, 'workspaces', 'foo') - // this simulates what it looks like when a user does i.e. npm pack from a workspace root + // this simulates what it looks like when a user does `npm pack` from a workspace root const files = await packlist({ path: root, prefix: root, @@ -98,6 +99,7 @@ t.test('packing a workspace root does not include children', async (t) => { 'package.json', ]) + // prove if we leave off workspaces we do not omit them const secondFiles = await packlist({ path: root, prefix: root,