Skip to content

Commit 1744a3c

Browse files
rosen-vladimirovrosen-vladimirov
authored
Merge pull request #5237 from NativeScript/vladimirov/fix-plugins-verification
fix: improve check for duplicated NativeScript plugins
2 parents 8e29a44 + 755752a commit 1744a3c

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)