From 755752a9d40760064331663b353ef965b8756496 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 3 Feb 2020 14:53:15 +0200 Subject: [PATCH] fix: improve check for duplicated NativeScript plugins Currently CLI fails when different versions of the same plugin are used. However, this introduces a lot of issues for Android, where the `nativescript-permissions` plugin is used by most of the other plugins. When they hardcode different version, CLI stops the build. However, this plugin is pure JS and does not have native part. To resolve this, separate the checks for iOS and Android. For Android, the SBG may fail in case different versions of the plugin are detected. However, there's no easy way to determine this, so just make the checks and show debug messages. For iOS the build will fail in case framework is linked multiple times from different locations, so group the plugins by frameworks and fail in case the same framework is listed multiple times by different plugins or by different versions of the same plugin --- lib/definitions/plugins.d.ts | 2 +- lib/services/ios-project-service.ts | 2 +- lib/services/metadata-filtering-service.ts | 2 +- lib/services/plugins-service.ts | 105 ++++++++++++++---- .../node-modules/node-modules-builder.ts | 2 +- test/plugins-service.ts | 64 ++++++++--- 6 files changed, 136 insertions(+), 41 deletions(-) diff --git a/lib/definitions/plugins.d.ts b/lib/definitions/plugins.d.ts index 9cc8be5360..6f70fa6518 100644 --- a/lib/definitions/plugins.d.ts +++ b/lib/definitions/plugins.d.ts @@ -4,7 +4,7 @@ interface IPluginsService { addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string): void; removeFromPackageJson(plugin: string, projectDir: string): void; getAllInstalledPlugins(projectData: IProjectData): Promise; - getAllProductionPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): IPluginData[]; + getAllProductionPlugins(projectData: IProjectData, platform: string, dependencies?: IDependencyData[]): IPluginData[]; ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise; /** diff --git a/lib/services/ios-project-service.ts b/lib/services/ios-project-service.ts index efc873f680..99c674b05c 100644 --- a/lib/services/ios-project-service.ts +++ b/lib/services/ios-project-service.ts @@ -471,7 +471,7 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ } private getAllProductionPlugins(projectData: IProjectData): IPluginData[] { - return (this.$injector.resolve("pluginsService")).getAllProductionPlugins(projectData); + return (this.$injector.resolve("pluginsService")).getAllProductionPlugins(projectData, this.getPlatformData(projectData).platformNameLowerCase); } private replace(name: string): string { diff --git a/lib/services/metadata-filtering-service.ts b/lib/services/metadata-filtering-service.ts index dbc3fb2a61..368cc5e2e1 100644 --- a/lib/services/metadata-filtering-service.ts +++ b/lib/services/metadata-filtering-service.ts @@ -23,7 +23,7 @@ export class MetadataFilteringService implements IMetadataFilteringService { if (nativeApiConfiguration) { const whitelistedItems: string[] = []; if (nativeApiConfiguration["whitelist-plugins-usages"]) { - const plugins = this.$pluginsService.getAllProductionPlugins(projectData); + const plugins = this.$pluginsService.getAllProductionPlugins(projectData, platform); for (const pluginData of plugins) { const pathToPlatformsDir = pluginData.pluginPlatformsFolderPath(platform); const pathToPluginsMetadataConfig = path.join(pathToPlatformsDir, MetadataFilteringConstants.NATIVE_API_USAGE_FILE_NAME); diff --git a/lib/services/plugins-service.ts b/lib/services/plugins-service.ts index 8c887a4740..14528c7d68 100644 --- a/lib/services/plugins-service.ts +++ b/lib/services/plugins-service.ts @@ -172,7 +172,7 @@ export class PluginsService implements IPluginsService { return _.filter(nodeModules, nodeModuleData => nodeModuleData && nodeModuleData.isPlugin); } - public getAllProductionPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): IPluginData[] { + public getAllProductionPlugins(projectData: IProjectData, platform: string, dependencies?: IDependencyData[]): IPluginData[] { dependencies = dependencies || this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir); if (_.isEmpty(dependencies)) { @@ -180,7 +180,7 @@ export class PluginsService implements IPluginsService { } let productionPlugins: IDependencyData[] = dependencies.filter(d => !!d.nativescript); - productionPlugins = this.ensureValidProductionPlugins(productionPlugins, projectData.projectDir); + productionPlugins = this.ensureValidProductionPlugins(productionPlugins, projectData.projectDir, platform); const pluginData = productionPlugins.map(plugin => this.convertToPluginData(plugin, projectData.projectDir)); return pluginData; } @@ -202,15 +202,27 @@ export class PluginsService implements IPluginsService { return pluginPackageJsonContent && pluginPackageJsonContent.nativescript; } - private ensureValidProductionPlugins = _.memoize<(productionDependencies: IDependencyData[], projectDir: string) => IDependencyData[]>(this._ensureValidProductionPlugins, (productionDependencies: IDependencyData[], projectDir: string) => { + private ensureValidProductionPlugins = _.memoize<(productionDependencies: IDependencyData[], projectDir: string, platform: string) => IDependencyData[]>(this._ensureValidProductionPlugins, (productionDependencies: IDependencyData[], projectDir: string, platform: string) => { let key = _.sortBy(productionDependencies, p => p.directory).map(d => JSON.stringify(d, null, 2)).join("\n"); - key += projectDir; + key += projectDir + platform; return key; }); - private _ensureValidProductionPlugins(productionDependencies: IDependencyData[], projectDir: string): IDependencyData[] { - const clonedProductionDependencies = _.cloneDeep(productionDependencies); - const dependenciesGroupedByName = _.groupBy(clonedProductionDependencies, p => p.name); + private _ensureValidProductionPlugins(productionDependencies: IDependencyData[], projectDir: string, platform: string): IDependencyData[] { + let clonedProductionDependencies = _.cloneDeep(productionDependencies); + platform = platform.toLowerCase(); + + if (this.$mobileHelper.isAndroidPlatform(platform)) { + this.ensureValidProductionPluginsForAndroid(clonedProductionDependencies); + } else if (this.$mobileHelper.isiOSPlatform(platform)) { + clonedProductionDependencies = this.ensureValidProductionPluginsForIOS(clonedProductionDependencies, projectDir, platform); + } + + return clonedProductionDependencies; + } + + private ensureValidProductionPluginsForAndroid(productionDependencies: IDependencyData[]): void { + const dependenciesGroupedByName = _.groupBy(productionDependencies, p => p.name); _.each(dependenciesGroupedByName, (dependencyOccurrences, dependencyName) => { if (dependencyOccurrences.length > 1) { // the dependency exists multiple times in node_modules @@ -218,31 +230,79 @@ export class PluginsService implements IPluginsService { const versions = _.keys(dependencyOccurrencesGroupedByVersion); if (versions.length === 1) { // all dependencies with this name have the same version - this.$logger.warn(`Detected same versions (${_.first(versions)}) of the ${dependencyName} installed at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}`); - const selectedPackage = _.minBy(dependencyOccurrences, d => d.depth); - this.$logger.info(`CLI will use only the native code from '${selectedPackage.directory}'.`.green); - _.each(dependencyOccurrences, dependency => { - if (dependency !== selectedPackage) { - clonedProductionDependencies.splice(clonedProductionDependencies.indexOf(dependency), 1); - } - }); + this.$logger.debug(`Detected same versions (${_.first(versions)}) of the ${dependencyName} installed at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}`); } else { - const message = this.getFailureMessageForDifferentDependencyVersions(dependencyName, dependencyOccurrencesGroupedByVersion, projectDir); - this.$errors.fail(message); + this.$logger.debug(`Detected different versions of the ${dependencyName} installed at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}\nThis can cause build failures.`); } } }); + } - return clonedProductionDependencies; + private ensureValidProductionPluginsForIOS(productionDependencies: IDependencyData[], projectDir: string, platform: string): IDependencyData[] { + const dependenciesWithFrameworks: any[] = []; + _.each(productionDependencies, d => { + const pathToPlatforms = path.join(d.directory, "platforms", platform); + if (this.$fs.exists(pathToPlatforms)) { + const contents = this.$fs.readDirectory(pathToPlatforms); + _.each(contents, file => { + if (path.extname(file) === ".framework") { + dependenciesWithFrameworks.push({ ...d, frameworkName: path.basename(file), frameworkLocation: path.join(pathToPlatforms, file) }); + } + }); + } + }); + + if (dependenciesWithFrameworks.length > 0) { + const dependenciesGroupedByFrameworkName = _.groupBy(dependenciesWithFrameworks, d => d.frameworkName); + _.each(dependenciesGroupedByFrameworkName, (dependencyOccurrences, frameworkName) => { + if (dependencyOccurrences.length > 1) { + // A framework exists multiple times in node_modules + const groupedByName = _.groupBy(dependencyOccurrences, d => d.name); + const pluginsNames = _.keys(groupedByName); + if (pluginsNames.length > 1) { + // fail - the same framework is installed by different dependencies. + const locations = dependencyOccurrences.map(d => d.frameworkLocation); + let msg = `Detected the framework ${frameworkName} is installed from multiple plugins at locations:\n${locations.join("\n")}\n`; + msg += this.getHelpMessage(projectDir); + this.$errors.fail(msg); + } + + const dependencyName = _.first(pluginsNames); + const dependencyOccurrencesGroupedByVersion = _.groupBy(dependencyOccurrences, g => g.version); + const versions = _.keys(dependencyOccurrencesGroupedByVersion); + if (versions.length === 1) { + // all dependencies with this name have the same version + this.$logger.warn(`Detected the framework ${frameworkName} is installed multiple times from the same versions of plugin (${_.first(versions)}) at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}`); + const selectedPackage = _.minBy(dependencyOccurrences, d => d.depth); + this.$logger.info(`CLI will use only the native code from '${selectedPackage.directory}'.`.green); + _.each(dependencyOccurrences, dependency => { + if (dependency !== selectedPackage) { + productionDependencies.splice(productionDependencies.indexOf(dependency), 1); + } + }); + } else { + const message = this.getFailureMessageForDifferentDependencyVersions(dependencyName, frameworkName, dependencyOccurrencesGroupedByVersion, projectDir); + this.$errors.fail(message); + } + } + }); + } + + return productionDependencies; } - private getFailureMessageForDifferentDependencyVersions(dependencyName: string, dependencyOccurrencesGroupedByVersion: IDictionary, projectDir: string): string { - let message = `Cannot use different versions of a NativeScript plugin in your application. -${dependencyName} plugin occurs multiple times in node_modules:\n`; + private getFailureMessageForDifferentDependencyVersions(dependencyName: string, frameworkName: string, dependencyOccurrencesGroupedByVersion: IDictionary, projectDir: string): string { + let message = `Cannot use the same framework ${frameworkName} multiple times in your application. +This framework comes from ${dependencyName} plugin, which is installed multiple times in node_modules:\n`; _.each(dependencyOccurrencesGroupedByVersion, (dependencies, version) => { message += dependencies.map(d => `* Path: ${d.directory}, version: ${d.version}\n`); }); + message += this.getHelpMessage(projectDir); + return message; + } + + private getHelpMessage(projectDir: string): string { const existingLockFiles: string[] = []; PluginsService.LOCK_FILES.forEach(lockFile => { if (this.$fs.exists(path.join(projectDir, lockFile))) { @@ -255,8 +315,7 @@ ${dependencyName} plugin occurs multiple times in node_modules:\n`; msgForLockFiles += ` and ${existingLockFiles.join(", ")}`; } - message += `Probably you need to update your dependencies, remove node_modules${msgForLockFiles} and try again.`; - return message; + return `\nProbably you need to update your dependencies, remove node_modules${msgForLockFiles} and try again.`; } private convertToPluginData(cacheData: IDependencyData | INodeModuleData, projectDir: string): IPluginData { diff --git a/lib/tools/node-modules/node-modules-builder.ts b/lib/tools/node-modules/node-modules-builder.ts index f67267299b..82ebc5b1ec 100644 --- a/lib/tools/node-modules/node-modules-builder.ts +++ b/lib/tools/node-modules/node-modules-builder.ts @@ -9,7 +9,7 @@ export class NodeModulesBuilder implements INodeModulesBuilder { const dependencies = this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir); await platformData.platformProjectService.beforePrepareAllPlugins(projectData, dependencies); - const pluginsData = this.$pluginsService.getAllProductionPlugins(projectData, dependencies); + const pluginsData = this.$pluginsService.getAllProductionPlugins(projectData, platformData.platformNameLowerCase, dependencies); if (_.isEmpty(pluginsData)) { return; } diff --git a/test/plugins-service.ts b/test/plugins-service.ts index bc7d220236..97bc7d2dda 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -586,7 +586,8 @@ describe("Plugins service", () => { unitTestsInjector.register("platformsDataService", {}); unitTestsInjector.register("filesHashService", {}); unitTestsInjector.register("fs", { - exists: (filePath: string) => false + exists: (filePath: string) => filePath.indexOf("ios") !== -1, + readDirectory: (dir: string) => dir.indexOf("nativescript-ui-core") !== -1 ? ["a.framework"] : [] }); unitTestsInjector.register("packageManager", {}); unitTestsInjector.register("options", {}); @@ -796,7 +797,7 @@ describe("Plugins service", () => { "version": "4.0.0", } ], - expectedWarning: "Detected same versions (4.0.0) of the nativescript-ui-core installed at locations: /Users/username/projectDir/node_modules/nativescript-ui-core, " + + expectedWarning: "Detected the framework a.framework is installed multiple times from the same versions of plugin (4.0.0) at locations: /Users/username/projectDir/node_modules/nativescript-ui-core, " + "/Users/username/projectDir/node_modules/nativescript-ui-listview/node_modules/nativescript-ui-core" }, { @@ -844,10 +845,45 @@ describe("Plugins service", () => { "dependencies": [] }, ], - expectedOutput: new Error(`Cannot use different versions of a NativeScript plugin in your application. -nativescript-ui-core plugin occurs multiple times in node_modules:\n` + + expectedOutput: new Error(`Cannot use the same framework a.framework multiple times in your application. +This framework comes from nativescript-ui-core plugin, which is installed multiple times in node_modules:\n` + "* Path: /Users/username/projectDir/node_modules/nativescript-ui-core, version: 3.0.0\n" + - "* Path: /Users/username/projectDir/node_modules/nativescript-ui-listview/node_modules/nativescript-ui-core, version: 4.0.0\n" + + "* Path: /Users/username/projectDir/node_modules/nativescript-ui-listview/node_modules/nativescript-ui-core, version: 4.0.0\n\n" + + `Probably you need to update your dependencies, remove node_modules and try again.`), + }, + { + testName: "fails when same framework is installed from multiple plugins", + inputDependencies: [ + { + "name": "nativescript-ui-core-forked", + "directory": "/Users/username/projectDir/node_modules/nativescript-ui-core-forked", + "depth": 0, + "version": "3.0.0", + "nativescript": { + "platforms": { + "android": "6.0.0", + "ios": "6.0.0" + } + }, + "dependencies": [] + }, + { + "name": "nativescript-ui-core", + "directory": "/Users/username/projectDir/node_modules/nativescript-ui-core", + "depth": 0, + "version": "3.0.0", + "nativescript": { + "platforms": { + "android": "6.0.0", + "ios": "6.0.0" + } + }, + "dependencies": [] + }, + ], + expectedOutput: new Error(`Detected the framework a.framework is installed from multiple plugins at locations:\n` + + "/Users/username/projectDir/node_modules/nativescript-ui-core-forked/platforms/ios/a.framework\n" + + "/Users/username/projectDir/node_modules/nativescript-ui-core/platforms/ios/a.framework\n\n" + `Probably you need to update your dependencies, remove node_modules and try again.`), } ]; @@ -858,9 +894,9 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` + const pluginsService: IPluginsService = unitTestsInjector.resolve(PluginsService); if (testCase.expectedOutput instanceof Error) { - assert.throws(() => pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, testCase.inputDependencies), testCase.expectedOutput.message); + assert.throws(() => pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, "ios", testCase.inputDependencies), testCase.expectedOutput.message); } else { - const plugins = pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, testCase.inputDependencies); + const plugins = pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, "ios", testCase.inputDependencies); if (testCase.expectedWarning) { const logger = unitTestsInjector.resolve("logger"); @@ -884,8 +920,8 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` + const pluginsService: IPluginsService = unitTestsInjector.resolve(PluginsService); const inputDependencies = [ { - "name": "tns-core-modules", - "directory": "/Users/username/projectDir/node_modules/tns-core-modules", + "name": "nativescript-ui-core", + "directory": "/Users/username/projectDir/node_modules/nativescript-ui-core", "depth": 0, "version": "6.3.0", "nativescript": { @@ -899,8 +935,8 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` + ] }, { - "name": "tns-core-modules", - "directory": "/Users/username/projectDir/node_modules/some-package/tns-core-modules", + "name": "nativescript-ui-core", + "directory": "/Users/username/projectDir/node_modules/some-package/nativescript-ui-core", "depth": 1, "version": "6.3.0", "nativescript": { @@ -916,16 +952,16 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` + ]; _.range(3).forEach(() => { - pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, inputDependencies); + pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, "ios", inputDependencies); }); const logger = unitTestsInjector.resolve("logger"); - const expectedWarnMessage = "Detected same versions (%s) of the tns-core-modules installed at locations: /Users/username/projectDir/node_modules/tns-core-modules, /Users/username/projectDir/node_modules/some-package/tns-core-modules\n"; + const expectedWarnMessage = "Detected the framework a.framework is installed multiple times from the same versions of plugin (%s) at locations: /Users/username/projectDir/node_modules/nativescript-ui-core, /Users/username/projectDir/node_modules/some-package/nativescript-ui-core\n"; assert.equal(logger.warnOutput, util.format(expectedWarnMessage, "6.3.0"), "The warn message must be shown only once - the result of the private method must be cached as input dependencies have not changed"); inputDependencies[0].version = "1.0.0"; inputDependencies[1].version = "1.0.0"; - pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, inputDependencies); + pluginsService.getAllProductionPlugins({ projectDir: "projectDir" }, "ios", inputDependencies); assert.equal(logger.warnOutput, util.format(expectedWarnMessage, "6.3.0") + util.format(expectedWarnMessage, "1.0.0"), "When something in input dependencies change, the cached value shouldn't be taken into account"); }); });