Skip to content

Commit 755752a

Browse files
rosen-vladimirovrosen-vladimirov
committed
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
1 parent 8e29a44 commit 755752a

File tree

6 files changed

+136
-41
lines changed

6 files changed

+136
-41
lines changed

‎lib/definitions/plugins.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ interface IPluginsService {
44
addToPackageJson(plugin: string, version: string, isDev: boolean, projectDir: string): void;
55
removeFromPackageJson(plugin: string, projectDir: string): void;
66
getAllInstalledPlugins(projectData: IProjectData): Promise<IPluginData[]>;
7-
getAllProductionPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): IPluginData[];
7+
getAllProductionPlugins(projectData: IProjectData, platform: string, dependencies?: IDependencyData[]): IPluginData[];
88
ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void>;
99

1010
/**

‎lib/services/ios-project-service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ
471471
}
472472

473473
private getAllProductionPlugins(projectData: IProjectData): IPluginData[] {
474-
return (<IPluginsService>this.$injector.resolve("pluginsService")).getAllProductionPlugins(projectData);
474+
return (<IPluginsService>this.$injector.resolve("pluginsService")).getAllProductionPlugins(projectData, this.getPlatformData(projectData).platformNameLowerCase);
475475
}
476476

477477
private replace(name: string): string {

‎lib/services/metadata-filtering-service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export class MetadataFilteringService implements IMetadataFilteringService {
2323
if (nativeApiConfiguration) {
2424
const whitelistedItems: string[] = [];
2525
if (nativeApiConfiguration["whitelist-plugins-usages"]) {
26-
const plugins = this.$pluginsService.getAllProductionPlugins(projectData);
26+
const plugins = this.$pluginsService.getAllProductionPlugins(projectData, platform);
2727
for (const pluginData of plugins) {
2828
const pathToPlatformsDir = pluginData.pluginPlatformsFolderPath(platform);
2929
const pathToPluginsMetadataConfig = path.join(pathToPlatformsDir, MetadataFilteringConstants.NATIVE_API_USAGE_FILE_NAME);

‎lib/services/plugins-service.ts

+82-23
Original file line numberDiff line numberDiff line change
@@ -172,15 +172,15 @@ export class PluginsService implements IPluginsService {
172172
return _.filter(nodeModules, nodeModuleData => nodeModuleData && nodeModuleData.isPlugin);
173173
}
174174

175-
public getAllProductionPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): IPluginData[] {
175+
public getAllProductionPlugins(projectData: IProjectData, platform: string, dependencies?: IDependencyData[]): IPluginData[] {
176176
dependencies = dependencies || this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir);
177177

178178
if (_.isEmpty(dependencies)) {
179179
return [];
180180
}
181181

182182
let productionPlugins: IDependencyData[] = dependencies.filter(d => !!d.nativescript);
183-
productionPlugins = this.ensureValidProductionPlugins(productionPlugins, projectData.projectDir);
183+
productionPlugins = this.ensureValidProductionPlugins(productionPlugins, projectData.projectDir, platform);
184184
const pluginData = productionPlugins.map(plugin => this.convertToPluginData(plugin, projectData.projectDir));
185185
return pluginData;
186186
}
@@ -202,47 +202,107 @@ export class PluginsService implements IPluginsService {
202202
return pluginPackageJsonContent && pluginPackageJsonContent.nativescript;
203203
}
204204

205-
private ensureValidProductionPlugins = _.memoize<(productionDependencies: IDependencyData[], projectDir: string) => IDependencyData[]>(this._ensureValidProductionPlugins, (productionDependencies: IDependencyData[], projectDir: string) => {
205+
private ensureValidProductionPlugins = _.memoize<(productionDependencies: IDependencyData[], projectDir: string, platform: string) => IDependencyData[]>(this._ensureValidProductionPlugins, (productionDependencies: IDependencyData[], projectDir: string, platform: string) => {
206206
let key = _.sortBy(productionDependencies, p => p.directory).map(d => JSON.stringify(d, null, 2)).join("\n");
207-
key += projectDir;
207+
key += projectDir + platform;
208208
return key;
209209
});
210210

211-
private _ensureValidProductionPlugins(productionDependencies: IDependencyData[], projectDir: string): IDependencyData[] {
212-
const clonedProductionDependencies = _.cloneDeep(productionDependencies);
213-
const dependenciesGroupedByName = _.groupBy(clonedProductionDependencies, p => p.name);
211+
private _ensureValidProductionPlugins(productionDependencies: IDependencyData[], projectDir: string, platform: string): IDependencyData[] {
212+
let clonedProductionDependencies = _.cloneDeep(productionDependencies);
213+
platform = platform.toLowerCase();
214+
215+
if (this.$mobileHelper.isAndroidPlatform(platform)) {
216+
this.ensureValidProductionPluginsForAndroid(clonedProductionDependencies);
217+
} else if (this.$mobileHelper.isiOSPlatform(platform)) {
218+
clonedProductionDependencies = this.ensureValidProductionPluginsForIOS(clonedProductionDependencies, projectDir, platform);
219+
}
220+
221+
return clonedProductionDependencies;
222+
}
223+
224+
private ensureValidProductionPluginsForAndroid(productionDependencies: IDependencyData[]): void {
225+
const dependenciesGroupedByName = _.groupBy(productionDependencies, p => p.name);
214226
_.each(dependenciesGroupedByName, (dependencyOccurrences, dependencyName) => {
215227
if (dependencyOccurrences.length > 1) {
216228
// the dependency exists multiple times in node_modules
217229
const dependencyOccurrencesGroupedByVersion = _.groupBy(dependencyOccurrences, g => g.version);
218230
const versions = _.keys(dependencyOccurrencesGroupedByVersion);
219231
if (versions.length === 1) {
220232
// all dependencies with this name have the same version
221-
this.$logger.warn(`Detected same versions (${_.first(versions)}) of the ${dependencyName} installed at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}`);
222-
const selectedPackage = _.minBy(dependencyOccurrences, d => d.depth);
223-
this.$logger.info(`CLI will use only the native code from '${selectedPackage.directory}'.`.green);
224-
_.each(dependencyOccurrences, dependency => {
225-
if (dependency !== selectedPackage) {
226-
clonedProductionDependencies.splice(clonedProductionDependencies.indexOf(dependency), 1);
227-
}
228-
});
233+
this.$logger.debug(`Detected same versions (${_.first(versions)}) of the ${dependencyName} installed at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}`);
229234
} else {
230-
const message = this.getFailureMessageForDifferentDependencyVersions(dependencyName, dependencyOccurrencesGroupedByVersion, projectDir);
231-
this.$errors.fail(message);
235+
this.$logger.debug(`Detected different versions of the ${dependencyName} installed at locations: ${_.map(dependencyOccurrences, d => d.directory).join(", ")}\nThis can cause build failures.`);
232236
}
233237
}
234238
});
239+
}
235240

236-
return clonedProductionDependencies;
241+
private ensureValidProductionPluginsForIOS(productionDependencies: IDependencyData[], projectDir: string, platform: string): IDependencyData[] {
242+
const dependenciesWithFrameworks: any[] = [];
243+
_.each(productionDependencies, d => {
244+
const pathToPlatforms = path.join(d.directory, "platforms", platform);
245+
if (this.$fs.exists(pathToPlatforms)) {
246+
const contents = this.$fs.readDirectory(pathToPlatforms);
247+
_.each(contents, file => {
248+
if (path.extname(file) === ".framework") {
249+
dependenciesWithFrameworks.push({ ...d, frameworkName: path.basename(file), frameworkLocation: path.join(pathToPlatforms, file) });
250+
}
251+
});
252+
}
253+
});
254+
255+
if (dependenciesWithFrameworks.length > 0) {
256+
const dependenciesGroupedByFrameworkName = _.groupBy(dependenciesWithFrameworks, d => d.frameworkName);
257+
_.each(dependenciesGroupedByFrameworkName, (dependencyOccurrences, frameworkName) => {
258+
if (dependencyOccurrences.length > 1) {
259+
// A framework exists multiple times in node_modules
260+
const groupedByName = _.groupBy(dependencyOccurrences, d => d.name);
261+
const pluginsNames = _.keys(groupedByName);
262+
if (pluginsNames.length > 1) {
263+
// fail - the same framework is installed by different dependencies.
264+
const locations = dependencyOccurrences.map(d => d.frameworkLocation);
265+
let msg = `Detected the framework ${frameworkName} is installed from multiple plugins at locations:\n${locations.join("\n")}\n`;
266+
msg += this.getHelpMessage(projectDir);
267+
this.$errors.fail(msg);
268+
}
269+
270+
const dependencyName = _.first(pluginsNames);
271+
const dependencyOccurrencesGroupedByVersion = _.groupBy(dependencyOccurrences, g => g.version);
272+
const versions = _.keys(dependencyOccurrencesGroupedByVersion);
273+
if (versions.length === 1) {
274+
// all dependencies with this name have the same version
275+
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(", ")}`);
276+
const selectedPackage = _.minBy(dependencyOccurrences, d => d.depth);
277+
this.$logger.info(`CLI will use only the native code from '${selectedPackage.directory}'.`.green);
278+
_.each(dependencyOccurrences, dependency => {
279+
if (dependency !== selectedPackage) {
280+
productionDependencies.splice(productionDependencies.indexOf(dependency), 1);
281+
}
282+
});
283+
} else {
284+
const message = this.getFailureMessageForDifferentDependencyVersions(dependencyName, frameworkName, dependencyOccurrencesGroupedByVersion, projectDir);
285+
this.$errors.fail(message);
286+
}
287+
}
288+
});
289+
}
290+
291+
return productionDependencies;
237292
}
238293

239-
private getFailureMessageForDifferentDependencyVersions(dependencyName: string, dependencyOccurrencesGroupedByVersion: IDictionary<IDependencyData[]>, projectDir: string): string {
240-
let message = `Cannot use different versions of a NativeScript plugin in your application.
241-
${dependencyName} plugin occurs multiple times in node_modules:\n`;
294+
private getFailureMessageForDifferentDependencyVersions(dependencyName: string, frameworkName: string, dependencyOccurrencesGroupedByVersion: IDictionary<IDependencyData[]>, projectDir: string): string {
295+
let message = `Cannot use the same framework ${frameworkName} multiple times in your application.
296+
This framework comes from ${dependencyName} plugin, which is installed multiple times in node_modules:\n`;
242297
_.each(dependencyOccurrencesGroupedByVersion, (dependencies, version) => {
243298
message += dependencies.map(d => `* Path: ${d.directory}, version: ${d.version}\n`);
244299
});
245300

301+
message += this.getHelpMessage(projectDir);
302+
return message;
303+
}
304+
305+
private getHelpMessage(projectDir: string): string {
246306
const existingLockFiles: string[] = [];
247307
PluginsService.LOCK_FILES.forEach(lockFile => {
248308
if (this.$fs.exists(path.join(projectDir, lockFile))) {
@@ -255,8 +315,7 @@ ${dependencyName} plugin occurs multiple times in node_modules:\n`;
255315
msgForLockFiles += ` and ${existingLockFiles.join(", ")}`;
256316
}
257317

258-
message += `Probably you need to update your dependencies, remove node_modules${msgForLockFiles} and try again.`;
259-
return message;
318+
return `\nProbably you need to update your dependencies, remove node_modules${msgForLockFiles} and try again.`;
260319
}
261320

262321
private convertToPluginData(cacheData: IDependencyData | INodeModuleData, projectDir: string): IPluginData {

‎lib/tools/node-modules/node-modules-builder.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export class NodeModulesBuilder implements INodeModulesBuilder {
99
const dependencies = this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir);
1010
await platformData.platformProjectService.beforePrepareAllPlugins(projectData, dependencies);
1111

12-
const pluginsData = this.$pluginsService.getAllProductionPlugins(projectData, dependencies);
12+
const pluginsData = this.$pluginsService.getAllProductionPlugins(projectData, platformData.platformNameLowerCase, dependencies);
1313
if (_.isEmpty(pluginsData)) {
1414
return;
1515
}

‎test/plugins-service.ts

+50-14
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,8 @@ describe("Plugins service", () => {
586586
unitTestsInjector.register("platformsDataService", {});
587587
unitTestsInjector.register("filesHashService", {});
588588
unitTestsInjector.register("fs", {
589-
exists: (filePath: string) => false
589+
exists: (filePath: string) => filePath.indexOf("ios") !== -1,
590+
readDirectory: (dir: string) => dir.indexOf("nativescript-ui-core") !== -1 ? ["a.framework"] : []
590591
});
591592
unitTestsInjector.register("packageManager", {});
592593
unitTestsInjector.register("options", {});
@@ -796,7 +797,7 @@ describe("Plugins service", () => {
796797
"version": "4.0.0",
797798
}
798799
],
799-
expectedWarning: "Detected same versions (4.0.0) of the nativescript-ui-core installed at locations: /Users/username/projectDir/node_modules/nativescript-ui-core, " +
800+
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, " +
800801
"/Users/username/projectDir/node_modules/nativescript-ui-listview/node_modules/nativescript-ui-core"
801802
},
802803
{
@@ -844,10 +845,45 @@ describe("Plugins service", () => {
844845
"dependencies": []
845846
},
846847
],
847-
expectedOutput: new Error(`Cannot use different versions of a NativeScript plugin in your application.
848-
nativescript-ui-core plugin occurs multiple times in node_modules:\n` +
848+
expectedOutput: new Error(`Cannot use the same framework a.framework multiple times in your application.
849+
This framework comes from nativescript-ui-core plugin, which is installed multiple times in node_modules:\n` +
849850
"* Path: /Users/username/projectDir/node_modules/nativescript-ui-core, version: 3.0.0\n" +
850-
"* Path: /Users/username/projectDir/node_modules/nativescript-ui-listview/node_modules/nativescript-ui-core, version: 4.0.0\n" +
851+
"* Path: /Users/username/projectDir/node_modules/nativescript-ui-listview/node_modules/nativescript-ui-core, version: 4.0.0\n\n" +
852+
`Probably you need to update your dependencies, remove node_modules and try again.`),
853+
},
854+
{
855+
testName: "fails when same framework is installed from multiple plugins",
856+
inputDependencies: [
857+
{
858+
"name": "nativescript-ui-core-forked",
859+
"directory": "/Users/username/projectDir/node_modules/nativescript-ui-core-forked",
860+
"depth": 0,
861+
"version": "3.0.0",
862+
"nativescript": {
863+
"platforms": {
864+
"android": "6.0.0",
865+
"ios": "6.0.0"
866+
}
867+
},
868+
"dependencies": []
869+
},
870+
{
871+
"name": "nativescript-ui-core",
872+
"directory": "/Users/username/projectDir/node_modules/nativescript-ui-core",
873+
"depth": 0,
874+
"version": "3.0.0",
875+
"nativescript": {
876+
"platforms": {
877+
"android": "6.0.0",
878+
"ios": "6.0.0"
879+
}
880+
},
881+
"dependencies": []
882+
},
883+
],
884+
expectedOutput: new Error(`Detected the framework a.framework is installed from multiple plugins at locations:\n` +
885+
"/Users/username/projectDir/node_modules/nativescript-ui-core-forked/platforms/ios/a.framework\n" +
886+
"/Users/username/projectDir/node_modules/nativescript-ui-core/platforms/ios/a.framework\n\n" +
851887
`Probably you need to update your dependencies, remove node_modules and try again.`),
852888
}
853889
];
@@ -858,9 +894,9 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` +
858894
const pluginsService: IPluginsService = unitTestsInjector.resolve(PluginsService);
859895

860896
if (testCase.expectedOutput instanceof Error) {
861-
assert.throws(() => pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, testCase.inputDependencies), testCase.expectedOutput.message);
897+
assert.throws(() => pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", testCase.inputDependencies), testCase.expectedOutput.message);
862898
} else {
863-
const plugins = pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, testCase.inputDependencies);
899+
const plugins = pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", testCase.inputDependencies);
864900

865901
if (testCase.expectedWarning) {
866902
const logger = unitTestsInjector.resolve<stubs.LoggerStub>("logger");
@@ -884,8 +920,8 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` +
884920
const pluginsService: IPluginsService = unitTestsInjector.resolve(PluginsService);
885921
const inputDependencies = [
886922
{
887-
"name": "tns-core-modules",
888-
"directory": "/Users/username/projectDir/node_modules/tns-core-modules",
923+
"name": "nativescript-ui-core",
924+
"directory": "/Users/username/projectDir/node_modules/nativescript-ui-core",
889925
"depth": 0,
890926
"version": "6.3.0",
891927
"nativescript": {
@@ -899,8 +935,8 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` +
899935
]
900936
},
901937
{
902-
"name": "tns-core-modules",
903-
"directory": "/Users/username/projectDir/node_modules/some-package/tns-core-modules",
938+
"name": "nativescript-ui-core",
939+
"directory": "/Users/username/projectDir/node_modules/some-package/nativescript-ui-core",
904940
"depth": 1,
905941
"version": "6.3.0",
906942
"nativescript": {
@@ -916,16 +952,16 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` +
916952
];
917953

918954
_.range(3).forEach(() => {
919-
pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, inputDependencies);
955+
pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", inputDependencies);
920956
});
921957

922958
const logger = unitTestsInjector.resolve<stubs.LoggerStub>("logger");
923959

924-
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";
960+
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";
925961
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");
926962
inputDependencies[0].version = "1.0.0";
927963
inputDependencies[1].version = "1.0.0";
928-
pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, inputDependencies);
964+
pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", inputDependencies);
929965
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");
930966
});
931967
});

0 commit comments

Comments
 (0)