Protect against runtimePackage.version returning null #5444

Activity

1. shirakaba commented on Dec 5, 2020

   

   shirakaba

   on Dec 5, 2020

   Author

   More actions

   Followed it a bit further, but haven't put any debug in yet.

   runtimePackage is determined by calling this.$projectDataService.getRuntimePackage().

   So things could be going wrong either in getRuntimePackage() or getInstalledRuntimePackage():

   nativescript-cli/lib/services/project-data-service.ts

   Lines 567 to 655 in 6de0c3e

   	public getRuntimePackage(
   	projectDir: string,
   	platform: constants.SupportedPlatform
   	): IBasePluginData {
   	platform = platform.toLowerCase() as constants.SupportedPlatform;
   	const packageJson = this.$fs.readJson(
   	path.join(projectDir, constants.PACKAGE_JSON_FILE_NAME)
   	);
   	const runtimeName =
   	platform === PlatformTypes.android
   	? constants.TNS_ANDROID_RUNTIME_NAME
   	: constants.TNS_IOS_RUNTIME_NAME;
   	if (
   	packageJson &&
   	packageJson.nativescript &&
   	packageJson.nativescript[runtimeName] &&
   	packageJson.nativescript[runtimeName].version
   	) {
   	// if we have a nativescript key with a runtime version in package.json
   	// that means we are dealing with a legacy project, and should respect
   	// that information
   	return {
   	name: runtimeName,
   	version: packageJson.nativescript[runtimeName].version,
   	};
   	}
   	return this.getInstalledRuntimePackage(projectDir, platform);
   	}
   	private getInstalledRuntimePackage(
   	projectDir: string,
   	platform: constants.SupportedPlatform
   	): IBasePluginData {
   	const runtimePackage = this.$pluginsService
   	.getDependenciesFromPackageJson(projectDir)
   	.devDependencies.find((d) => {
   	if (platform === constants.PlatformTypes.ios) {
   	return [
   	constants.SCOPED_IOS_RUNTIME_NAME,
   	constants.TNS_IOS_RUNTIME_NAME,
   	].includes(d.name);
   	} else if (platform === constants.PlatformTypes.android) {
   	return [
   	constants.SCOPED_ANDROID_RUNTIME_NAME,
   	constants.TNS_ANDROID_RUNTIME_NAME,
   	].includes(d.name);
   	}
   	});
   	if (runtimePackage) {
   	// in case we are using a local tgz for the runtime
   	//
   	if (runtimePackage.version.includes("tgz")) {
   	try {
   	const runtimePackageJsonPath = require.resolve(
   	`${runtimePackage.name}/package.json`,
   	{
   	paths: [projectDir],
   	}
   	);
   	runtimePackage.version = this.$fs.readJson(
   	runtimePackageJsonPath
   	).version;
   	} catch (err) {
   	runtimePackage.version = null;
   	}
   	}
   	return runtimePackage;
   	}
   	// default to the scoped runtimes
   	this.$logger.trace(
   	"Could not find an installed runtime, falling back to default runtimes"
   	);
   	if (platform === constants.PlatformTypes.ios) {
   	return {
   	name: constants.SCOPED_IOS_RUNTIME_NAME,
   	version: null,
   	};
   	} else if (platform === constants.PlatformTypes.android) {
   	return {
   	name: constants.SCOPED_ANDROID_RUNTIME_NAME,
   	version: null,
   	};
   	}
   	}
2. shirakaba commented on Dec 5, 2020

   

   shirakaba

   on Dec 5, 2020

   Author

   More actions

   Getting closer. It strictly expects the runtimes to be in devDependencies. Why is that?

   nativescript-cli/lib/services/project-data-service.ts

   Lines 602 to 616 in 6de0c3e

   	const runtimePackage = this.$pluginsService
   	.getDependenciesFromPackageJson(projectDir)
   	.devDependencies.find((d) => {
   	if (platform === constants.PlatformTypes.ios) {
   	return [
   	constants.SCOPED_IOS_RUNTIME_NAME,
   	constants.TNS_IOS_RUNTIME_NAME,
   	].includes(d.name);
   	} else if (platform === constants.PlatformTypes.android) {
   	return [
   	constants.SCOPED_ANDROID_RUNTIME_NAME,
   	constants.TNS_ANDROID_RUNTIME_NAME,
   	].includes(d.name);
   	}
   	});

   Moving them into devDependencies, my error is now refreshingly different:

   Invalid Version: file:../../node_modules/@nativescript/android

   This is pretty fragile. Now I see why the plugin seed doesn't hoist dependencies.

   Could we not have CLI find the version more robustly, e.g. by doing proper node module resolution and utilising the filesystem instead?

   By fixing this, we'd likely become able to hoist the runtime dependencies in monorepos, saving many megabytes for everyone.

   👍React with 👍1Reacted by Rob
3. farfromrefug commented on Jan 27, 2021

   

   farfromrefug

   on Jan 27, 2021

   Contributor

   More actions

   @shirakaba just stumbled upon this and wanted to react on your point about saving data with monorepo.
   you can already "achieve" that with pnpm. I have been using it with N for years and it saves me literally Gb of data! thus saving my hard drive too!

   👍React with 👍1Reacted by Jamie Birch