From 08a2c060ed6ebba6729735e20ebe789b64a3288d Mon Sep 17 00:00:00 2001 From: Bunyanuch Saengnet <53788417+bunysae@users.noreply.github.com> Date: Thu, 29 Oct 2020 14:49:46 +0000 Subject: [PATCH] Show files added since the last release and not part of the package (#456) Co-authored-by: Sindre Sorhus --- .gitmodules | 3 + integration-test | 1 + package.json | 5 +- readme.md | 4 + source/git-util.js | 16 +++ source/npm/util.js | 104 +++++++++++++++++- source/ui.js | 24 ++++ source/util.js | 7 ++ test/fixtures/npmignore/.hg | 1 + test/fixtures/npmignore/.npmignore | 2 + test/fixtures/npmignore/README.txt | 1 + test/fixtures/npmignore/readme.md | 1 + test/fixtures/npmignore/source/ignore.txt | 1 + .../npmignore/source/pay_attention.txt | 1 + test/fixtures/npmignore/test/file.txt | 1 + test/fixtures/package/.hg | 1 + test/fixtures/package/package.json | 3 + test/fixtures/package/source/ignore.txt | 1 + .../fixtures/package/source/pay_attention.txt | 1 + test/fixtures/readme.md | 2 + test/integration.js | 11 ++ test/npmignore.js | 94 ++++++++++++++++ 22 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 .gitmodules create mode 160000 integration-test create mode 100644 test/fixtures/npmignore/.hg create mode 100644 test/fixtures/npmignore/.npmignore create mode 100644 test/fixtures/npmignore/README.txt create mode 100644 test/fixtures/npmignore/readme.md create mode 100644 test/fixtures/npmignore/source/ignore.txt create mode 100644 test/fixtures/npmignore/source/pay_attention.txt create mode 100644 test/fixtures/npmignore/test/file.txt create mode 100644 test/fixtures/package/.hg create mode 100644 test/fixtures/package/package.json create mode 100644 test/fixtures/package/source/ignore.txt create mode 100644 test/fixtures/package/source/pay_attention.txt create mode 100644 test/fixtures/readme.md create mode 100644 test/integration.js create mode 100644 test/npmignore.js diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 00000000..18f98e71 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "integration-test"] + path = integration-test + url = https://github.com/bunysae/np_integration_test diff --git a/integration-test b/integration-test new file mode 160000 index 00000000..ad5e6e37 --- /dev/null +++ b/integration-test @@ -0,0 +1 @@ +Subproject commit ad5e6e3776cb3e2b396e7d3f5a6a7a4b5fa0b83e diff --git a/package.json b/package.json index 56fd5fbe..1ee9d715 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,7 @@ "github-url-from-git": "^1.5.0", "has-yarn": "^2.1.0", "hosted-git-info": "^3.0.0", + "ignore-walk": "^3.0.3", "import-local": "^3.0.2", "inquirer": "^7.0.0", "is-installed-globally": "^0.3.1", @@ -51,6 +52,7 @@ "listr-input": "^0.2.1", "log-symbols": "^3.0.0", "meow": "^6.0.0", + "minimatch": "^3.0.4", "new-github-release-url": "^1.0.0", "npm-name": "^6.0.0", "onetime": "^5.1.0", @@ -77,7 +79,8 @@ }, "ava": { "files": [ - "!test/fixtures" + "!test/fixtures", + "!integration-test" ] } } diff --git a/readme.md b/readme.md index 73e723b9..0984eb1c 100644 --- a/readme.md +++ b/readme.md @@ -280,6 +280,10 @@ Host * If you're running into other issues when using SSH, please consult [GitHub's support article](https://help.github.com/articles/connecting-to-github-with-ssh/). +### Ignore strategy + +The [ignore strategy](https://docs.npmjs.com/files/package.json#files), either maintained in the `files`-property in `package.json` or in `.npmignore`, is meant to help reduce the package size. To avoid broken packages caused by essential files being accidentally ignored, `np` prints out all the new and unpublished files added to Git. Test files and other [common files](https://docs.npmjs.com/files/package.json#files) that are never published are not considered. `np` assumes either a standard directory layout or a customized layout represented in the `directories` property in `package.json`. + ## FAQ ### I get an error when publishing my package through Yarn diff --git a/source/git-util.js b/source/git-util.js index b885fd7a..9ebfbf5f 100644 --- a/source/git-util.js +++ b/source/git-util.js @@ -1,6 +1,8 @@ 'use strict'; const execa = require('execa'); const escapeStringRegexp = require('escape-string-regexp'); +const ignoreWalker = require('ignore-walk'); +const pkgDir = require('pkg-dir'); const {verifyRequirementSatisfied} = require('./version'); exports.latestTag = async () => { @@ -8,6 +10,20 @@ exports.latestTag = async () => { return stdout; }; +exports.newFilesSinceLastRelease = async () => { + try { + const {stdout} = await execa('git', ['diff', '--stat', '--diff-filter=A', await this.latestTag(), 'HEAD']); + const result = stdout.trim().split('\n').slice(0, -1).map(row => row.slice(0, row.indexOf('|')).trim()); + return result; + } catch (_) { + // Get all files under version control + return ignoreWalker({ + path: pkgDir.sync(), + ignoreFiles: ['.gitignore'] + }); + } +}; + const firstCommit = async () => { const {stdout} = await execa('git', ['rev-list', '--max-parents=0', 'HEAD']); return stdout; diff --git a/source/npm/util.js b/source/npm/util.js index f7011b2d..93371291 100644 --- a/source/npm/util.js +++ b/source/npm/util.js @@ -7,6 +7,8 @@ const ow = require('ow'); const npmName = require('npm-name'); const chalk = require('chalk'); const pkgDir = require('pkg-dir'); +const ignoreWalker = require('ignore-walk'); +const minimatch = require('minimatch'); const {verifyRequirementSatisfied} = require('../version'); exports.checkConnection = () => pTimeout( @@ -117,16 +119,110 @@ exports.verifyRecentNpmVersion = async () => { }; exports.checkIgnoreStrategy = ({files}) => { - const rootDir = pkgDir.sync(); - const npmignoreExists = fs.existsSync(path.resolve(rootDir, '.npmignore')); - - if (!files && !npmignoreExists) { + if (!files && !npmignoreExistsInPackageRootDir()) { console.log(` \n${chalk.bold.yellow('Warning:')} No ${chalk.bold.cyan('files')} field specified in ${chalk.bold.magenta('package.json')} nor is a ${chalk.bold.magenta('.npmignore')} file present. Having one of those will prevent you from accidentally publishing development-specific files along with your package's source code to npm. `); } }; +function npmignoreExistsInPackageRootDir() { + const rootDir = pkgDir.sync(); + return fs.existsSync(path.resolve(rootDir, '.npmignore')); +} + +async function getFilesIgnoredByDotnpmignore(pkg, fileList) { + const whiteList = await ignoreWalker({ + path: pkgDir.sync(), + ignoreFiles: ['.npmignore'] + }); + return fileList.filter(minimatch.filter(getIgnoredFilesGlob(whiteList, pkg.directories), {matchBase: true, dot: true})); +} + +function getFilesNotIncludedInFilesProperty(pkg, fileList) { + const globArrayForFilesAndDirectories = [...pkg.files]; + const rootDir = pkgDir.sync(); + for (const glob of pkg.files) { + try { + if (fs.statSync(path.resolve(rootDir, glob)).isDirectory()) { + globArrayForFilesAndDirectories.push(`${glob}/**/*`); + } + } catch (_) {} + } + + const result = fileList.filter(minimatch.filter(getIgnoredFilesGlob(globArrayForFilesAndDirectories, pkg.directories), {matchBase: true, dot: true})); + return result.filter(minimatch.filter(getDefaultIncludedFilesGlob(pkg.main), {nocase: true, matchBase: true})); +} + +function getDefaultIncludedFilesGlob(mainFile) { + // According to https://docs.npmjs.com/files/package.json#files + // npm's default behavior is to always include these files. + const filesAlwaysIncluded = [ + 'package.json', + 'README*', + 'CHANGES*', + 'CHANGELOG*', + 'HISTORY*', + 'LICENSE*', + 'LICENCE*', + 'NOTICE*' + ]; + if (mainFile) { + filesAlwaysIncluded.push(mainFile); + } + + return `!{${filesAlwaysIncluded}}`; +} + +function getIgnoredFilesGlob(globArrayFromFilesProperty, packageDirectories) { + // According to https://docs.npmjs.com/files/package.json#files + // npm's default behavior is to ignore these files. + const filesIgnoredByDefault = [ + '.*.swp', + '.npmignore', + '.gitignore', + '._*', + '.DS_Store', + '.hg', + '.npmrc', + '.lock-wscript', + '.svn', + '.wafpickle-N', + '*.orig', + 'config.gypi', + 'CVS', + 'node_modules/**/*', + 'npm-debug.log', + 'package-lock.json', + '.git/**/*', + '.git' + ]; + + // Test files are assumed not to be part of the package + let testDirectoriesGlob = ''; + if (packageDirectories && Array.isArray(packageDirectories.test)) { + testDirectoriesGlob = packageDirectories.test.join(','); + } else if (packageDirectories && typeof packageDirectories.test === 'string') { + testDirectoriesGlob = packageDirectories.test; + } else { + // Fallback to `test` directory + testDirectoriesGlob = 'test/**/*'; + } + + return `!{${globArrayFromFilesProperty.join(',')},${filesIgnoredByDefault.join(',')},${testDirectoriesGlob}}`; +} + +// Get all files which will be ignored by either `.npmignore` or the `files` property in `package.json` (if defined). +exports.getNewAndUnpublishedFiles = async (pkg, newFiles = []) => { + if (pkg.files) { + return getFilesNotIncludedInFilesProperty(pkg, newFiles); + } + + if (npmignoreExistsInPackageRootDir()) { + return getFilesIgnoredByDotnpmignore(pkg, newFiles); + } +}; + exports.getRegistryUrl = async (pkgManager, pkg) => { const args = ['config', 'get', 'registry']; if (exports.isExternalRegistry(pkg)) { diff --git a/source/ui.js b/source/ui.js index 6c82aa27..569be234 100644 --- a/source/ui.js +++ b/source/ui.js @@ -50,6 +50,22 @@ const printCommitLog = async (repoUrl, registryUrl) => { }; }; +const checkIgnoredFiles = async pkg => { + const ignoredFiles = await util.getNewAndUnpublishedFiles(pkg); + if (!ignoredFiles || ignoredFiles.length === 0) { + return true; + } + + const answers = await inquirer.prompt([{ + type: 'confirm', + name: 'confirm', + message: `The following new files are not already part of your published package:\n${chalk.reset(ignoredFiles.map(path => `- ${path}`).join('\n'))}\nContinue?`, + default: false + }]); + + return answers.confirm; +}; + module.exports = async (options, pkg) => { const oldVersion = pkg.version; const extraBaseUrls = ['gitlab.com']; @@ -59,6 +75,14 @@ module.exports = async (options, pkg) => { if (options.runPublish) { checkIgnoreStrategy(pkg); + + const answerIgnoredFiles = await checkIgnoredFiles(pkg); + if (!answerIgnoredFiles) { + return { + ...options, + confirm: answerIgnoredFiles + }; + } } console.log(`\nPublish a new version of ${chalk.bold.magenta(pkg.name)} ${chalk.dim(`(current: ${oldVersion})`)}\n`); diff --git a/source/util.js b/source/util.js index c1762ea3..12a9c988 100644 --- a/source/util.js +++ b/source/util.js @@ -6,6 +6,8 @@ const execa = require('execa'); const pMemoize = require('p-memoize'); const ow = require('ow'); const pkgDir = require('pkg-dir'); +const gitUtil = require('./git-util'); +const npmUtil = require('./npm/util'); exports.readPkg = packagePath => { packagePath = packagePath ? pkgDir.sync(packagePath) : pkgDir.sync(); @@ -69,6 +71,11 @@ exports.getTagVersionPrefix = pMemoize(async options => { } }); +exports.getNewAndUnpublishedFiles = async pkg => { + const listNewFiles = await gitUtil.newFilesSinceLastRelease(); + return npmUtil.getNewAndUnpublishedFiles(pkg, listNewFiles); +}; + exports.getPreReleasePrefix = pMemoize(async options => { ow(options, ow.object.hasKeys('yarn')); diff --git a/test/fixtures/npmignore/.hg b/test/fixtures/npmignore/.hg new file mode 100644 index 00000000..3f06d3e2 --- /dev/null +++ b/test/fixtures/npmignore/.hg @@ -0,0 +1 @@ +should be ignored by default diff --git a/test/fixtures/npmignore/.npmignore b/test/fixtures/npmignore/.npmignore new file mode 100644 index 00000000..501c21cd --- /dev/null +++ b/test/fixtures/npmignore/.npmignore @@ -0,0 +1,2 @@ +ignore.txt +test diff --git a/test/fixtures/npmignore/README.txt b/test/fixtures/npmignore/README.txt new file mode 100644 index 00000000..5086e7b4 --- /dev/null +++ b/test/fixtures/npmignore/README.txt @@ -0,0 +1 @@ +File is always included in package. diff --git a/test/fixtures/npmignore/readme.md b/test/fixtures/npmignore/readme.md new file mode 100644 index 00000000..5086e7b4 --- /dev/null +++ b/test/fixtures/npmignore/readme.md @@ -0,0 +1 @@ +File is always included in package. diff --git a/test/fixtures/npmignore/source/ignore.txt b/test/fixtures/npmignore/source/ignore.txt new file mode 100644 index 00000000..26ef7633 --- /dev/null +++ b/test/fixtures/npmignore/source/ignore.txt @@ -0,0 +1 @@ +Ignore this file diff --git a/test/fixtures/npmignore/source/pay_attention.txt b/test/fixtures/npmignore/source/pay_attention.txt new file mode 100644 index 00000000..01a573f9 --- /dev/null +++ b/test/fixtures/npmignore/source/pay_attention.txt @@ -0,0 +1 @@ +File is excluded from .npmignore diff --git a/test/fixtures/npmignore/test/file.txt b/test/fixtures/npmignore/test/file.txt new file mode 100644 index 00000000..375fb8ee --- /dev/null +++ b/test/fixtures/npmignore/test/file.txt @@ -0,0 +1 @@ +ignore this file diff --git a/test/fixtures/package/.hg b/test/fixtures/package/.hg new file mode 100644 index 00000000..3f06d3e2 --- /dev/null +++ b/test/fixtures/package/.hg @@ -0,0 +1 @@ +should be ignored by default diff --git a/test/fixtures/package/package.json b/test/fixtures/package/package.json new file mode 100644 index 00000000..b2861deb --- /dev/null +++ b/test/fixtures/package/package.json @@ -0,0 +1,3 @@ +{ + "files": ["pay_attention.txt"] +} diff --git a/test/fixtures/package/source/ignore.txt b/test/fixtures/package/source/ignore.txt new file mode 100644 index 00000000..40f91d34 --- /dev/null +++ b/test/fixtures/package/source/ignore.txt @@ -0,0 +1 @@ +File is excluded from package.json diff --git a/test/fixtures/package/source/pay_attention.txt b/test/fixtures/package/source/pay_attention.txt new file mode 100644 index 00000000..e5f3e01f --- /dev/null +++ b/test/fixtures/package/source/pay_attention.txt @@ -0,0 +1 @@ +File in included in package.json diff --git a/test/fixtures/readme.md b/test/fixtures/readme.md new file mode 100644 index 00000000..c0816aa2 --- /dev/null +++ b/test/fixtures/readme.md @@ -0,0 +1,2 @@ +The directory is for the resources +in the script npmignore.js diff --git a/test/integration.js b/test/integration.js new file mode 100644 index 00000000..c486a9a3 --- /dev/null +++ b/test/integration.js @@ -0,0 +1,11 @@ +const test = require('ava'); +const execa = require('execa'); + +test.after.always(async () => { + await execa('git', ['submodule', 'update', '--remote']); +}); + +test('Integration tests', async t => { + await execa('ava', {cwd: 'integration-test'}); + t.pass(); +}); diff --git a/test/npmignore.js b/test/npmignore.js new file mode 100644 index 00000000..2769916a --- /dev/null +++ b/test/npmignore.js @@ -0,0 +1,94 @@ +import path from 'path'; +import test from 'ava'; +import proxyquire from 'proxyquire'; + +const newFiles = [ + 'source/ignore.txt', + 'source/pay_attention.txt', + '.hg', + 'test/file.txt', + 'readme.md', + 'README.txt' +]; + +test('ignored files using file-attribute in package.json with one file', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'package') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({files: ['pay_attention.txt']}, newFiles), ['source/ignore.txt']); +}); + +test('ignored file using file-attribute in package.json with directory', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'package') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({files: ['source']}, newFiles), []); +}); + +test('ignored test files using files attribute and directory structure in package.json', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'package') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({files: ['source'], directories: {test: 'test-tap'}}, newFiles), ['test/file.txt']); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({files: ['source'], directories: {test: ['test-tap']}}, newFiles), ['test/file.txt']); +}); + +test('ignored files using .npmignore', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'npmignore') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({name: 'npmignore'}, newFiles), ['source/ignore.txt']); +}); + +test('ignored test files using files attribute and .npmignore', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'npmignore') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({directories: {test: 'test-tap'}}, newFiles), ['source/ignore.txt', 'test/file.txt']); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({directories: {test: ['test-tap']}}, newFiles), ['source/ignore.txt', 'test/file.txt']); +}); + +test('dot files using files attribute', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'package') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({files: ['source']}, ['test/.dotfile']), []); +}); + +test('dot files using .npmignore', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures', 'npmignore') + } + }); + t.deepEqual(await testedModule.getNewAndUnpublishedFiles({}, ['test/.dot']), []); +}); + +test('ignore strategy is not used', async t => { + const testedModule = proxyquire('../source/npm/util', { + 'pkg-dir': + { + sync: () => path.resolve('test', 'fixtures') + } + }); + t.is(await testedModule.getNewAndUnpublishedFiles({name: 'no ignore strategy'}, newFiles), undefined); +});