Get package version from manifest

[Gudahtt]

The package version is now obtained from the package manifest rather than from the npm_package_version environment variable. This strategy works better with the --root flag, which might sometimes specify a root project directory that is different from the current working directory. The current version should always match the version in the manifest at the project root.

[legobeat]

Suggested change

	if (error.code === 'ENOENT') {
	exitWithError(
	`Package manifest not found at path: '${manifestPath}'\nRun this script from the project root directory, or set the project directory using the '--root' flag.`,
	);
	return;
	} else if (error.code === 'EACCES') {
	exitWithError(
	`Access to package manifest is forbidden by file access permissions: '${manifestPath}'`,
	);
	return;
	} else if (error.name === 'SyntaxError') {
	exitWithError(
	`Package manifest cannot be parsed as JSON: '${manifestPath}'`,
	);
	return;
	}
	if (error.code === 'ENOENT') {
	return exitWithError(
	`Package manifest not found at path: '${manifestPath}'\nRun this script from the project root directory, or set the project directory using the '--root' flag.`,
	);
	}
	if (error.code === 'EACCES') {
	return exitWithError(
	`Access to package manifest is forbidden by file access permissions: '${manifestPath}'`,
	);
	}
	if (error.name === 'SyntaxError') {
	return exitWithError(
	`Package manifest cannot be parsed as JSON: '${manifestPath}'`,
	);
	}
	throw err;

[legobeat]

Suggested change

	exitWithError(
	`Version not found. Please set the --currentVersion flag, or run this as an npm script from a project with the 'version' field set.`,
	);
	return;
	} else if (currentVersion && semver.valid(currentVersion) === null) {
	exitWithError(`Current version is not valid SemVer: '${currentVersion}'`);
	return;
	} else if (!repoUrl) {
	exitWithError(
	`npm package repository URL not found. Please set the '--repo' flag, or run this as an npm script from a project with the 'repository' field set.`,
	);
	return;
	} else if (!isValidUrl(repoUrl)) {
	exitWithError(`Invalid repo URL: '${repoUrl}'`);
	return;
	}
	return exitWithError(
	`Version not found. Please set the --currentVersion flag, or run this as an npm script from a project with the 'version' field set.`,
	);
	}
	if (semver.valid(currentVersion) === null) {
	return exitWithError(`Current version is not valid SemVer: '${currentVersion}'`);
	}
	if (!repoUrl) {
	return exitWithError(
	`npm package repository URL not found. Please set the '--repo' flag, or run this as an npm script from a project with the 'repository' field set.`,
	);
	}
	if (!isValidUrl(repoUrl)) {
	return exitWithError(`Invalid repo URL: '${repoUrl}'`);
	}

[Gudahtt]

The pattern of placing the return on the following line was to make complying with the ESLint "consistent-return" rule easier.

Now that you're pointing it out, I think I agree that this style is harder to read and more error prone. I can adjust that in a follow-up PR and find a better solution for complying with that lint rule. I'd rather avoid doing it here because the same pattern is used in more places, and I'd like to change them all at once. It's distinct from the main purpose of this PR as well.

[Gudahtt]

Done in #122

[legobeat]

Note in particular rethrowing errors that aren't explicitly caught.

[Gudahtt]

Oh darn, great find! Not re-throwing the error is a bad one. I've just fixed that in 41cb1e6

The requested style changes I can address in a follow-up PR

[legobeat]

lgtm!