Skip to content

fix: improve check for duplicated NativeScript plugins #5237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 3, 2020
Merged

fix: improve check for duplicated NativeScript plugins #5237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/definitions/plugins.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<IPluginData[]>;
getAllProductionPlugins(projectData: IProjectData, dependencies?: IDependencyData[]): IPluginData[];
getAllProductionPlugins(projectData: IProjectData, platform: string, dependencies?: IDependencyData[]): IPluginData[];
ensureAllDependenciesAreInstalled(projectData: IProjectData): Promise<void>;

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/services/ios-project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ export class IOSProjectService extends projectServiceBaseLib.PlatformProjectServ
}

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

private replace(name: string): string {
Expand Down
2 changes: 1 addition & 1 deletion lib/services/metadata-filtering-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
105 changes: 82 additions & 23 deletions lib/services/plugins-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,15 @@ 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)) {
return [];
}

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;
}
Expand All @@ -202,47 +202,107 @@ 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
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 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<IDependencyData[]>, 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<IDependencyData[]>, 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))) {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion lib/tools/node-modules/node-modules-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
64 changes: 50 additions & 14 deletions test/plugins-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", {});
Expand Down Expand Up @@ -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"
},
{
Expand Down Expand Up @@ -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.`),
}
];
Expand All @@ -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(<any>{ projectDir: "projectDir" }, testCase.inputDependencies), testCase.expectedOutput.message);
assert.throws(() => pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", testCase.inputDependencies), testCase.expectedOutput.message);
} else {
const plugins = pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, testCase.inputDependencies);
const plugins = pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", testCase.inputDependencies);

if (testCase.expectedWarning) {
const logger = unitTestsInjector.resolve<stubs.LoggerStub>("logger");
Expand All @@ -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": {
Expand All @@ -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": {
Expand All @@ -916,16 +952,16 @@ nativescript-ui-core plugin occurs multiple times in node_modules:\n` +
];

_.range(3).forEach(() => {
pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, inputDependencies);
pluginsService.getAllProductionPlugins(<any>{ projectDir: "projectDir" }, "ios", inputDependencies);
});

const logger = unitTestsInjector.resolve<stubs.LoggerStub>("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(<any>{ projectDir: "projectDir" }, inputDependencies);
pluginsService.getAllProductionPlugins(<any>{ 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");
});
});
Expand Down