Skip to content

Commit

Permalink
Improve startup: ensure package.json is only parsed once (#688)
Browse files Browse the repository at this point in the history
  • Loading branch information
tommy-mitchell authored Apr 6, 2023
1 parent 42a5095 commit eba203f
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 89 deletions.
14 changes: 7 additions & 7 deletions source/cli-implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import updateNotifier from 'update-notifier';
import hasYarn from 'has-yarn';
import {gracefulExit} from 'exit-hook';
import config from './config.js';
import * as util from './util.js';
import * as git from './git-util.js';
import {isPackageNameAvailable} from './npm/util.js';
import * as npm from './npm/util.js';
import Version from './version.js';
import * as util from './util.js';
import ui from './ui.js';
import np from './index.js';

Expand Down Expand Up @@ -99,7 +99,7 @@ const cli = meow(`
updateNotifier({pkg: cli.pkg}).notify();

try {
const {pkg, pkgPath} = await util.readPkg();
const {pkg, rootDir} = await util.readPkg(cli.flags.contents);

const defaultFlags = {
cleanup: true,
Expand All @@ -110,7 +110,7 @@ try {
'2fa': true,
};

const localConfig = await config();
const localConfig = await config(rootDir);

const flags = {
...defaultFlags,
Expand All @@ -125,7 +125,7 @@ try {

const runPublish = !flags.releaseDraftOnly && flags.publish && !pkg.private;

const availability = flags.publish ? await isPackageNameAvailable(pkg) : {
const availability = flags.publish ? await npm.isPackageNameAvailable(pkg) : {
isAvailable: false,
isUnknown: false,
};
Expand All @@ -140,14 +140,14 @@ try {
version,
runPublish,
branch,
}, {pkg, pkgPath});
}, {pkg, rootDir});

if (!options.confirm) {
gracefulExit();
}

console.log(); // Prints a newline for readability
const newPkg = await np(options.version, options);
const newPkg = await np(options.version, options, {pkg, rootDir});

if (options.preview || options.releaseDraftOnly) {
gracefulExit();
Expand Down
5 changes: 2 additions & 3 deletions source/config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import os from 'node:os';
import isInstalledGlobally from 'is-installed-globally';
import {packageDirectory} from 'pkg-dir';
import {cosmiconfig} from 'cosmiconfig';

// TODO: remove when cosmiconfig/cosmiconfig#283 lands
Expand All @@ -9,8 +8,8 @@ const loadESM = async filepath => {
return module.default ?? module;
};

const getConfig = async () => {
const searchDir = isInstalledGlobally ? os.homedir() : await packageDirectory();
const getConfig = async rootDir => {
const searchDir = isInstalledGlobally ? os.homedir() : rootDir;
const searchPlaces = ['.np-config.json', '.np-config.js', '.np-config.cjs', '.np-config.mjs'];
if (!isInstalledGlobally) {
searchPlaces.push('package.json');
Expand Down
16 changes: 4 additions & 12 deletions source/git-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import path from 'node:path';
import {execa} from 'execa';
import escapeStringRegexp from 'escape-string-regexp';
import ignoreWalker from 'ignore-walk';
import {packageDirectory} from 'pkg-dir';
import Version from './version.js';

export const latestTag = async () => {
Expand All @@ -15,7 +14,7 @@ export const root = async () => {
return stdout;
};

export const newFilesSinceLastRelease = async () => {
export const newFilesSinceLastRelease = async rootDir => {
try {
const {stdout} = await execa('git', ['diff', '--name-only', '--diff-filter=A', await latestTag(), 'HEAD']);
if (stdout.trim().length === 0) {
Expand All @@ -27,7 +26,7 @@ export const newFilesSinceLastRelease = async () => {
} catch {
// Get all files under version control
return ignoreWalker({
path: await packageDirectory(),
path: rootDir,
ignoreFiles: ['.gitignore'],
});
}
Expand Down Expand Up @@ -205,12 +204,7 @@ export const tagExistsOnRemote = async tagName => {

async function hasLocalBranch(branch) {
try {
await execa('git', [
'show-ref',
'--verify',
'--quiet',
`refs/heads/${branch}`,
]);
await execa('git', ['show-ref', '--verify', '--quiet', `refs/heads/${branch}`]);
return true;
} catch {
return false;
Expand All @@ -225,9 +219,7 @@ export const defaultBranch = async () => {
}
}

throw new Error(
'Could not infer the default Git branch. Please specify one with the --branch flag or with a np config.',
);
throw new Error('Could not infer the default Git branch. Please specify one with the --branch flag or with a np config.');
};

export const verifyTagDoesNotExistOnRemote = async tagName => {
Expand Down
7 changes: 2 additions & 5 deletions source/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import Listr from 'listr';
import {merge, throwError, catchError, filter, finalize} from 'rxjs';
import {readPackageUp} from 'read-pkg-up';
import hasYarn from 'has-yarn';
import {packageDirectorySync} from 'pkg-dir';
import hostedGitInfo from 'hosted-git-info';
import onetime from 'onetime';
import {asyncExitHook} from 'exit-hook';
Expand All @@ -15,10 +14,10 @@ import prerequisiteTasks from './prerequisite-tasks.js';
import gitTasks from './git-tasks.js';
import publish from './npm/publish.js';
import enable2fa from './npm/enable-2fa.js';
import * as npm from './npm/util.js';
import releaseTaskHelper from './release-task-helper.js';
import * as util from './util.js';
import * as git from './git-util.js';
import * as npm from './npm/util.js';

const exec = (cmd, args) => {
// Use `Observable` support if merged https://github.com/sindresorhus/execa/pull/26
Expand All @@ -28,7 +27,7 @@ const exec = (cmd, args) => {
};

// eslint-disable-next-line complexity
const np = async (input = 'patch', options) => {
const np = async (input = 'patch', options, {pkg, rootDir}) => {
if (!hasYarn() && options.yarn) {
throw new Error('Could not use Yarn without yarn.lock file');
}
Expand All @@ -38,12 +37,10 @@ const np = async (input = 'patch', options) => {
options.cleanup = false;
}

const {pkg} = await util.readPkg(options.contents);
const runTests = options.tests && !options.yolo;
const runCleanup = options.cleanup && !options.yolo;
const pkgManager = options.yarn === true ? 'yarn' : 'npm';
const pkgManagerName = options.yarn === true ? 'Yarn' : 'npm';
const rootDir = packageDirectorySync();
const hasLockFile = fs.existsSync(path.resolve(rootDir, options.yarn ? 'yarn.lock' : 'package-lock.json')) || fs.existsSync(path.resolve(rootDir, 'npm-shrinkwrap.json'));
const isOnGitHub = options.repoUrl && hostedGitInfo.fromUrl(options.repoUrl)?.type === 'github';
const testScript = options.testScript || 'test';
Expand Down
14 changes: 5 additions & 9 deletions source/npm/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import pTimeout from 'p-timeout';
import ow from 'ow';
import npmName from 'npm-name';
import chalk from 'chalk';
import {packageDirectory} from 'pkg-dir';
import semver from 'semver';
import Version from '../version.js';

Expand Down Expand Up @@ -129,21 +128,18 @@ export const verifyRecentNpmVersion = async () => {
Version.verifyRequirementSatisfied('npm', npmVersion);
};

const npmignoreExistsInPackageRootDir = async () => {
const rootDir = await packageDirectory();
return pathExists(path.resolve(rootDir, '.npmignore'));
};
export const checkIgnoreStrategy = ({files}, rootDir) => {
const npmignoreExistsInPackageRootDir = pathExists(path.resolve(rootDir, '.npmignore'));

export const checkIgnoreStrategy = async ({files}) => {
if (!files && !(await npmignoreExistsInPackageRootDir())) {
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.
`);
}
};

export const getFilesToBePacked = async () => {
const {stdout} = await execa('npm', ['pack', '--dry-run', '--json'], {cwd: await packageDirectory()});
export const getFilesToBePacked = async rootDir => {
const {stdout} = await execa('npm', ['pack', '--dry-run', '--json'], {cwd: rootDir});

const {files} = JSON.parse(stdout).at(0);
return files.map(file => file.path);
Expand Down
20 changes: 10 additions & 10 deletions source/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import isScoped from 'is-scoped';
import isInteractive from 'is-interactive';
import * as util from './util.js';
import * as git from './git-util.js';
import * as npmUtil from './npm/util.js';
import * as npm from './npm/util.js';
import Version from './version.js';
import prettyVersionDiff from './pretty-version-diff.js';

Expand Down Expand Up @@ -78,9 +78,9 @@ const printCommitLog = async (repoUrl, registryUrl, fromLatestTag, releaseBranch
};
};

const checkNewFilesAndDependencies = async (pkg, pkgPath) => {
const newFiles = await util.getNewFiles(pkg);
const newDependencies = await util.getNewDependencies(pkg, pkgPath);
const checkNewFilesAndDependencies = async (pkg, rootDir) => {
const newFiles = await util.getNewFiles(rootDir);
const newDependencies = await util.getNewDependencies(pkg, rootDir);

const noNewUnpublishedFiles = !newFiles.unpublished || newFiles.unpublished.length === 0;
const noNewFirstTimeFiles = !newFiles.firstTime || newFiles.firstTime.length === 0;
Expand Down Expand Up @@ -121,18 +121,18 @@ const checkNewFilesAndDependencies = async (pkg, pkgPath) => {
};

// eslint-disable-next-line complexity
const ui = async (options, {pkg, pkgPath}) => {
const ui = async (options, {pkg, rootDir}) => {
const oldVersion = pkg.version;
const extraBaseUrls = ['gitlab.com'];
const repoUrl = pkg.repository && githubUrlFromGit(pkg.repository.url, {extraBaseUrls});
const pkgManager = options.yarn ? 'yarn' : 'npm';
const registryUrl = await npmUtil.getRegistryUrl(pkgManager, pkg);
const registryUrl = await npm.getRegistryUrl(pkgManager, pkg);
const releaseBranch = options.branch;

if (options.runPublish) {
await npmUtil.checkIgnoreStrategy(pkg);
npm.checkIgnoreStrategy(pkg, rootDir);

const answerIgnoredFiles = await checkNewFilesAndDependencies(pkg, pkgPath);
const answerIgnoredFiles = await checkNewFilesAndDependencies(pkg, rootDir);
if (!answerIgnoredFiles) {
return {
...options,
Expand Down Expand Up @@ -253,7 +253,7 @@ const ui = async (options, {pkg, pkgPath}) => {
message: 'How should this pre-release version be tagged in npm?',
when: answers => options.runPublish && (Version.isPrereleaseOrIncrement(answers.customVersion) || Version.isPrereleaseOrIncrement(answers.version)) && !options.tag,
async choices() {
const existingPrereleaseTags = await npmUtil.prereleaseTags(pkg.name);
const existingPrereleaseTags = await npm.prereleaseTags(pkg.name);

return [
...existingPrereleaseTags,
Expand Down Expand Up @@ -283,7 +283,7 @@ const ui = async (options, {pkg, pkgPath}) => {
},
publishScoped: {
type: 'confirm',
when: isScoped(pkg.name) && options.availability.isAvailable && !options.availability.isUnknown && options.runPublish && (pkg.publishConfig && pkg.publishConfig.access !== 'restricted') && !npmUtil.isExternalRegistry(pkg),
when: isScoped(pkg.name) && options.availability.isAvailable && !options.availability.isUnknown && options.runPublish && (pkg.publishConfig && pkg.publishConfig.access !== 'restricted') && !npm.isExternalRegistry(pkg),
message: `This scoped repo ${chalk.bold.magenta(pkg.name)} hasn't been published. Do you want to publish it publicly?`,
default: false,
},
Expand Down
19 changes: 10 additions & 9 deletions source/util.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import path from 'node:path';
import {readPackageUp} from 'read-pkg-up';
import issueRegex from 'issue-regex';
import terminalLink from 'terminal-link';
Expand All @@ -6,20 +7,20 @@ import pMemoize from 'p-memoize';
import ow from 'ow';
import chalk from 'chalk';
import {packageDirectory} from 'pkg-dir';
import * as gitUtil from './git-util.js';
import * as npmUtil from './npm/util.js';
import * as git from './git-util.js';
import * as npm from './npm/util.js';

export const readPkg = async packagePath => {
packagePath = packagePath ? await packageDirectory(packagePath) : await packageDirectory();
if (!packagePath) {
throw new Error('No `package.json` found. Make sure the current directory is a valid package.');
}

const {packageJson, path} = await readPackageUp({
const {packageJson, path: pkgPath} = await readPackageUp({
cwd: packagePath,
});

return {pkg: packageJson, pkgPath: path};
return {pkg: packageJson, rootDir: path.dirname(pkgPath)};
};

export const linkifyIssues = (url, message) => {
Expand Down Expand Up @@ -72,18 +73,18 @@ export const getTagVersionPrefix = pMemoize(async options => {

export const joinList = list => chalk.reset(list.map(item => `- ${item}`).join('\n'));

export const getNewFiles = async () => {
const listNewFiles = await gitUtil.newFilesSinceLastRelease();
const listPkgFiles = await npmUtil.getFilesToBePacked();
export const getNewFiles = async rootDir => {
const listNewFiles = await git.newFilesSinceLastRelease(rootDir);
const listPkgFiles = await npm.getFilesToBePacked(rootDir);

return {
unpublished: listNewFiles.filter(file => !listPkgFiles.includes(file) && !file.startsWith('.git')),
firstTime: listNewFiles.filter(file => listPkgFiles.includes(file)),
};
};

export const getNewDependencies = async (newPkg, pkgPath) => {
let oldPkg = await gitUtil.readFileFromLastRelease(pkgPath);
export const getNewDependencies = async (newPkg, rootDir) => {
let oldPkg = await git.readFileFromLastRelease(path.resolve(rootDir, 'package.json'));
oldPkg = JSON.parse(oldPkg);

const newDependencies = [];
Expand Down
6 changes: 2 additions & 4 deletions test/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ const getConfigsWhenGlobalBinaryIsUsed = async homedirStub => {
const promises = pathsPkgDir.map(async pathPkgDir => {
const getConfig = await esmock(testedModulePath, {
'is-installed-globally': true,
'pkg-dir': {packageDirectory: async () => pathPkgDir},
'node:os': {homedir: homedirStub},
});
return getConfig();
return getConfig(pathPkgDir);
});

return Promise.all(promises);
Expand All @@ -29,10 +28,9 @@ const getConfigsWhenLocalBinaryIsUsed = async pathPkgDir => {
const promises = homedirs.map(async homedir => {
const getConfig = await esmock(testedModulePath, {
'is-installed-globally': false,
'pkg-dir': {packageDirectory: async () => pathPkgDir},
'node:os': {homedir: () => homedir},
});
return getConfig();
return getConfig(pathPkgDir);
});

return Promise.all(promises);
Expand Down
11 changes: 7 additions & 4 deletions test/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import test from 'ava';
import sinon from 'sinon';
import esmock from 'esmock';
import * as util from '../source/util.js';
import np from '../source/index.js';

const defaultOptions = {
Expand All @@ -15,9 +16,11 @@ const defaultOptions = {
renderer: 'silent',
};

const npPkg = await util.readPkg();

const npFails = test.macro(async (t, inputs, message) => {
await t.throwsAsync(
Promise.all(inputs.map(input => np(input, defaultOptions))),
Promise.all(inputs.map(input => np(input, defaultOptions, npPkg))),
{message},
);
});
Expand Down Expand Up @@ -51,15 +54,15 @@ test('skip enabling 2FA if the package exists', async t => {
},
'../source/npm/enable-2fa.js': enable2faStub,
'../source/npm/publish.js': sinon.stub().returns({pipe: sinon.stub()}),
}, {});
});

await t.notThrowsAsync(npMock('1.0.0', {
...defaultOptions,
availability: {
isAvailable: false,
isUnknown: false,
},
}));
}, npPkg));

t.true(enable2faStub.notCalled);
});
Expand Down Expand Up @@ -87,7 +90,7 @@ test('skip enabling 2FA if the `2fa` option is false', async t => {
isUnknown: false,
},
'2fa': false,
}));
}, npPkg));

t.true(enable2faStub.notCalled);
});
Loading

0 comments on commit eba203f

Please sign in to comment.