Skip to content

Commit 7c9c635

Browse files
author
Fatme
authored
Merge pull request #3904 from NativeScript/fatme/js-plugins
chore: don't show warnings for {N} plugins without native code when bundle option is provided
2 parents 47d79e8 + a2952bc commit 7c9c635

File tree

6 files changed

+236
-17
lines changed

6 files changed

+236
-17
lines changed

‎lib/definitions/plugins.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ interface IPluginsService {
1515
validate(platformData: IPlatformData, projectData: IProjectData): Promise<void>;
1616
preparePluginNativeCode(pluginData: IPluginData, platform: string, projectData: IProjectData): Promise<void>;
1717
convertToPluginData(cacheData: any, projectDir: string): IPluginData;
18+
isNativeScriptPlugin(pluginName: string, projectData: IProjectData): boolean;
1819
}
1920

2021
interface IPackageJsonDepedenciesResult {

‎lib/definitions/preview-app-livesync.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ declare global {
1818
}
1919

2020
interface IPreviewAppPluginsService {
21-
comparePluginsOnDevice(device: Device): Promise<void>;
21+
comparePluginsOnDevice(data: IPreviewAppLiveSyncData, device: Device): Promise<void>;
2222
getExternalPlugins(device: Device): string[];
2323
}
2424

‎lib/services/livesync/playground/preview-app-livesync-service.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export class PreviewAppLiveSyncService implements IPreviewAppLiveSyncService {
4949
startSyncFilesTimeout: startSyncFilesTimeout.bind(this)
5050
}
5151
});
52-
await this.$previewAppPluginsService.comparePluginsOnDevice(device);
52+
await this.$previewAppPluginsService.comparePluginsOnDevice(data, device);
5353
const payloads = await this.syncFilesForPlatformSafe(data, device.platform);
5454

5555
return payloads;
@@ -60,7 +60,7 @@ export class PreviewAppLiveSyncService implements IPreviewAppLiveSyncService {
6060
this.showWarningsForNativeFiles(files);
6161

6262
for (const device of this.$previewSdkService.connectedDevices) {
63-
await this.$previewAppPluginsService.comparePluginsOnDevice(device);
63+
await this.$previewAppPluginsService.comparePluginsOnDevice(data, device);
6464
}
6565

6666
const platforms = _(this.$previewSdkService.connectedDevices)

‎lib/services/livesync/playground/preview-app-plugins-service.ts

+23-3
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,26 @@ import * as semver from "semver";
33
import * as util from "util";
44
import { Device } from "nativescript-preview-sdk";
55
import { PluginComparisonMessages } from "./preview-app-constants";
6+
import { NODE_MODULES_DIR_NAME } from "../../../common/constants";
7+
import { PLATFORMS_DIR_NAME } from "../../../constants";
68

79
export class PreviewAppPluginsService implements IPreviewAppPluginsService {
810
private previewAppVersionWarnings: IDictionary<string[]> = {};
911

1012
constructor(private $fs: IFileSystem,
1113
private $logger: ILogger,
14+
private $pluginsService: IPluginsService,
1215
private $projectData: IProjectData) { }
1316

14-
public async comparePluginsOnDevice(device: Device): Promise<void> {
17+
public async comparePluginsOnDevice(data: IPreviewAppLiveSyncData, device: Device): Promise<void> {
1518
if (!this.previewAppVersionWarnings[device.previewAppVersion]) {
1619
const devicePlugins = this.getDevicePlugins(device);
1720
const localPlugins = this.getLocalPlugins();
1821
const warnings = _.keys(localPlugins)
1922
.map(localPlugin => {
2023
const localPluginVersion = localPlugins[localPlugin];
2124
const devicePluginVersion = devicePlugins[localPlugin];
22-
return this.getWarningForPlugin(localPlugin, localPluginVersion, devicePluginVersion, device.id);
25+
return this.getWarningForPlugin(data, localPlugin, localPluginVersion, devicePluginVersion, device);
2326
})
2427
.filter(item => !!item);
2528
this.previewAppVersionWarnings[device.previewAppVersion] = warnings;
@@ -62,7 +65,15 @@ export class PreviewAppPluginsService implements IPreviewAppPluginsService {
6265
}
6366
}
6467

65-
private getWarningForPlugin(localPlugin: string, localPluginVersion: string, devicePluginVersion: string, deviceId: string): string {
68+
private getWarningForPlugin(data: IPreviewAppLiveSyncData, localPlugin: string, localPluginVersion: string, devicePluginVersion: string, device: Device): string {
69+
if (data && data.appFilesUpdaterOptions && data.appFilesUpdaterOptions.bundle && this.isNativeScriptPluginWithoutNativeCode(localPlugin, device.platform)) {
70+
return null;
71+
}
72+
73+
return this.getWarningForPluginCore(localPlugin, localPluginVersion, devicePluginVersion, device.id);
74+
}
75+
76+
private getWarningForPluginCore(localPlugin: string, localPluginVersion: string, devicePluginVersion: string, deviceId: string): string {
6677
this.$logger.trace(`Comparing plugin ${localPlugin} with localPluginVersion ${localPluginVersion} and devicePluginVersion ${devicePluginVersion}`);
6778

6879
if (devicePluginVersion) {
@@ -80,5 +91,14 @@ export class PreviewAppPluginsService implements IPreviewAppPluginsService {
8091

8192
return util.format(PluginComparisonMessages.PLUGIN_NOT_INCLUDED_IN_PREVIEW_APP, localPlugin, deviceId);
8293
}
94+
95+
private isNativeScriptPluginWithoutNativeCode(localPlugin: string, platform: string): boolean {
96+
return this.$pluginsService.isNativeScriptPlugin(localPlugin, this.$projectData) && !this.hasNativeCode(localPlugin, platform);
97+
}
98+
99+
private hasNativeCode(localPlugin: string, platform: string): boolean {
100+
const nativeFolderPath = path.join(this.$projectData.projectDir, NODE_MODULES_DIR_NAME, localPlugin, PLATFORMS_DIR_NAME, platform.toLowerCase());
101+
return this.$fs.exists(nativeFolderPath) && !this.$fs.isEmptyDir(nativeFolderPath);
102+
}
83103
}
84104
$injector.register("previewAppPluginsService", PreviewAppPluginsService);

‎lib/services/plugins-service.ts

+7
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as path from "path";
22
import * as shelljs from "shelljs";
33
import * as semver from "semver";
44
import * as constants from "../constants";
5+
import { NODE_MODULES_DIR_NAME } from "../common/constants";
56

67
export class PluginsService implements IPluginsService {
78
private static INSTALL_COMMAND_NAME = "install";
@@ -207,6 +208,12 @@ export class PluginsService implements IPluginsService {
207208
};
208209
}
209210

211+
public isNativeScriptPlugin(pluginName: string, projectData: IProjectData): boolean {
212+
const pluginPackageJsonPath = path.join(projectData.projectDir, NODE_MODULES_DIR_NAME, pluginName, "package.json");
213+
const pluginPackageJsonContent = this.$fs.readJson(pluginPackageJsonPath);
214+
return pluginPackageJsonContent && pluginPackageJsonContent.nativescript;
215+
}
216+
210217
private getBasicPluginInformation(dependencies: any): IBasePluginData[] {
211218
return _.map(dependencies, (version: string, key: string) => ({
212219
name: key,

‎test/services/playground/preview-app-plugins-service.ts

+202-11
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,48 @@ import { PluginComparisonMessages } from "../../../lib/services/livesync/playgro
88
let readJsonParams: string[] = [];
99
let warnParams: string[] = [];
1010

11-
function createTestInjector(localPlugins: IStringDictionary): IInjector {
11+
const deviceId = "myTestDeviceId";
12+
const projectDir = "testProjectDir";
13+
14+
function createTestInjector(localPlugins: IStringDictionary, options?: { isNativeScriptPlugin?: boolean, hasPluginNativeCode?: boolean }): IInjector {
15+
options = options || {};
1216
const injector = new Yok();
1317
injector.register("fs", {
1418
readJson: (filePath: string) => {
1519
readJsonParams.push(filePath);
16-
return {
20+
const result: any = {
1721
dependencies: localPlugins
1822
};
23+
24+
if (options.isNativeScriptPlugin) {
25+
result.nativescript = {};
26+
}
27+
28+
return result;
29+
},
30+
exists: (filePath: string) => {
31+
return options.hasPluginNativeCode;
32+
},
33+
isEmptyDir: (filePath: string) => {
34+
return !options.hasPluginNativeCode;
35+
}
36+
});
37+
injector.register("pluginsService", {
38+
isNativeScriptPlugin: () => {
39+
return options.isNativeScriptPlugin;
1940
}
2041
});
2142
injector.register("logger", {
2243
trace: () => ({}),
2344
warn: (message: string) => warnParams.push(message)
2445
});
2546
injector.register("projectData", {
26-
projectDir: "testProjectDir"
47+
projectDir
2748
});
2849
injector.register("previewAppPluginsService", PreviewAppPluginsService);
2950
return injector;
3051
}
3152

32-
const deviceId = "myTestDeviceId";
33-
3453
function createDevice(plugins: string): Device {
3554
return {
3655
id: deviceId,
@@ -45,8 +64,20 @@ function createDevice(plugins: string): Device {
4564
};
4665
}
4766

48-
function setup(localPlugins: IStringDictionary, previewAppPlugins: IStringDictionary): any {
49-
const injector = createTestInjector(localPlugins);
67+
function createPreviewLiveSyncData(options?: { bundle: boolean }) {
68+
return {
69+
projectDir,
70+
appFilesUpdaterOptions: {
71+
release: false,
72+
bundle: options.bundle
73+
},
74+
env: {}
75+
};
76+
}
77+
78+
function setup(localPlugins: IStringDictionary, previewAppPlugins: IStringDictionary,
79+
options?: { isNativeScriptPlugin: boolean, hasPluginNativeCode: boolean }): any {
80+
const injector = createTestInjector(localPlugins, options);
5081
const previewAppPluginsService = injector.resolve("previewAppPluginsService");
5182
const device = createDevice(JSON.stringify(previewAppPlugins));
5283

@@ -57,7 +88,7 @@ function setup(localPlugins: IStringDictionary, previewAppPlugins: IStringDictio
5788
}
5889

5990
describe("previewAppPluginsService", () => {
60-
describe("comparePluginsOnDevice", () => {
91+
describe("comparePluginsOnDevice without bundle", () => {
6192
it("should persist warnings per preview app's version", async () => {
6293
const localPlugins = {
6394
"nativescript-facebook": "2.2.3",
@@ -84,7 +115,9 @@ describe("previewAppPluginsService", () => {
84115
return originalGetLocalPlugins.apply(previewAppPluginsService);
85116
};
86117

87-
await previewAppPluginsService.comparePluginsOnDevice(createDevice(JSON.stringify(previewAppPlugins)));
118+
const previewLiveSyncData = createPreviewLiveSyncData({ bundle: false });
119+
120+
await previewAppPluginsService.comparePluginsOnDevice(previewLiveSyncData, createDevice(JSON.stringify(previewAppPlugins)));
88121

89122
const expectedWarnings = [
90123
util.format(PluginComparisonMessages.PLUGIN_NOT_INCLUDED_IN_PREVIEW_APP, "nativescript-facebook", deviceId),
@@ -98,7 +131,7 @@ describe("previewAppPluginsService", () => {
98131
isGetLocalPluginsCalled = false;
99132
warnParams = [];
100133

101-
await previewAppPluginsService.comparePluginsOnDevice(createDevice(JSON.stringify(previewAppPlugins)));
134+
await previewAppPluginsService.comparePluginsOnDevice(previewLiveSyncData, createDevice(JSON.stringify(previewAppPlugins)));
102135

103136
assert.isFalse(isGetDevicePluginsCalled);
104137
assert.isFalse(isGetLocalPluginsCalled);
@@ -225,13 +258,171 @@ describe("previewAppPluginsService", () => {
225258
it(`${testCase.name}`, async () => {
226259
const { previewAppPluginsService, device } = setup(testCase.localPlugins, testCase.previewAppPlugins);
227260

228-
await previewAppPluginsService.comparePluginsOnDevice(device);
261+
await previewAppPluginsService.comparePluginsOnDevice(createPreviewLiveSyncData({ bundle: false }), device);
229262

230263
assert.equal(warnParams.length, testCase.expectedWarnings.length);
231264
testCase.expectedWarnings.forEach(warning => assert.include(warnParams, warning));
232265
});
233266
}
234267
});
268+
describe("comparePluginsOnDevice with bundle", () => {
269+
const testCases = [
270+
{
271+
name: "should show warning for non nativescript plugin that has lower major version",
272+
localPlugins: {
273+
lodash: "1.2.3"
274+
},
275+
previewAppPlugins: {
276+
lodash: "2.3.3"
277+
},
278+
isNativeScriptPlugin: false,
279+
hasPluginNativeCode: false,
280+
expectedWarnings: [
281+
util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "lodash", "1.2.3", "2.3.3")
282+
]
283+
},
284+
{
285+
name: "should show warning for non nativescript plugin that has greather major version",
286+
localPlugins: {
287+
lodash: "3.2.3"
288+
},
289+
previewAppPlugins: {
290+
lodash: "2.3.3"
291+
},
292+
isNativeScriptPlugin: false,
293+
hasPluginNativeCode: false,
294+
expectedWarnings: [
295+
util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "lodash", "3.2.3", "2.3.3")
296+
]
297+
},
298+
{
299+
name: "should show warning for non nativescript plugin that has greather minor version",
300+
localPlugins: {
301+
lodash: "3.4.5"
302+
},
303+
previewAppPlugins: {
304+
lodash: "3.3.0"
305+
},
306+
isNativeScriptPlugin: false,
307+
hasPluginNativeCode: false,
308+
expectedWarnings: [
309+
util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_GREATHER_MINOR_VERSION, "lodash", "3.4.5", "3.3.0")
310+
]
311+
},
312+
{
313+
name: "should not show warning for non nativescript plugin that has the same version",
314+
localPlugins: {
315+
lodash: "3.4.5"
316+
},
317+
previewAppPlugins: {
318+
lodash: "3.4.5"
319+
},
320+
isNativeScriptPlugin: false,
321+
hasPluginNativeCode: false,
322+
expectedWarnings: []
323+
},
324+
{
325+
name: "should not show warning for nativescript plugin without native code that has lower major version",
326+
localPlugins: {
327+
"nativescript-theme-core": "2.4.5"
328+
},
329+
previewAppPlugins: {
330+
"nativescript-theme-core": "3.4.5"
331+
},
332+
isNativeScriptPlugin: true,
333+
hasPluginNativeCode: false,
334+
expectedWarnings: <string[]>[]
335+
},
336+
{
337+
name: "should not show warning for nativescript plugin without native code that has greather major version",
338+
localPlugins: {
339+
"nativescript-theme-core": "4.4.5"
340+
},
341+
previewAppPlugins: {
342+
"nativescript-theme-core": "3.4.5"
343+
},
344+
isNativeScriptPlugin: true,
345+
hasPluginNativeCode: false,
346+
expectedWarnings: []
347+
},
348+
{
349+
name: "should not show warning for nativescript plugin without native code that has greather minor version",
350+
localPlugins: {
351+
"nativescript-theme-core": "4.6.5"
352+
},
353+
previewAppPlugins: {
354+
"nativescript-theme-core": "4.4.5"
355+
},
356+
isNativeScriptPlugin: true,
357+
hasPluginNativeCode: false,
358+
expectedWarnings: []
359+
},
360+
{
361+
name: "should not show warning for nativescript plugin without native code that has the same version",
362+
localPlugins: {
363+
"nativescript-theme-core": "4.6.5"
364+
},
365+
previewAppPlugins: {
366+
"nativescript-theme-core": "4.6.5"
367+
},
368+
isNativeScriptPlugin: true,
369+
hasPluginNativeCode: false,
370+
expectedWarnings: []
371+
},
372+
{
373+
name: "should show warning for nativescript plugin with native code that has lower major version",
374+
localPlugins: {
375+
"nativescript-theme-core": "2.4.5"
376+
},
377+
previewAppPlugins: {
378+
"nativescript-theme-core": "3.4.5"
379+
},
380+
isNativeScriptPlugin: true,
381+
hasPluginNativeCode: true,
382+
expectedWarnings: [util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "nativescript-theme-core", "2.4.5", "3.4.5")]
383+
},
384+
{
385+
name: "should show warning for nativescript plugin with native code that has greather major version",
386+
localPlugins: {
387+
"nativescript-theme-core": "4.4.5"
388+
},
389+
previewAppPlugins: {
390+
"nativescript-theme-core": "3.4.5"
391+
},
392+
isNativeScriptPlugin: true,
393+
hasPluginNativeCode: true,
394+
expectedWarnings: [util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "nativescript-theme-core", "4.4.5", "3.4.5")]
395+
},
396+
{
397+
name: "should show warning for nativescript plugin with native code that has greather minor version",
398+
localPlugins: {
399+
"nativescript-theme-core": "4.4.5"
400+
},
401+
previewAppPlugins: {
402+
"nativescript-theme-core": "4.3.5"
403+
},
404+
isNativeScriptPlugin: true,
405+
hasPluginNativeCode: true,
406+
expectedWarnings: [util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_GREATHER_MINOR_VERSION, "nativescript-theme-core", "4.4.5", "4.3.5")]
407+
},
408+
];
409+
410+
afterEach(() => {
411+
warnParams = [];
412+
readJsonParams = [];
413+
});
414+
415+
_.each(testCases, testCase => {
416+
it(`${testCase.name}`, async () => {
417+
const { previewAppPluginsService, device } = setup(testCase.localPlugins, testCase.previewAppPlugins, { isNativeScriptPlugin: testCase.isNativeScriptPlugin, hasPluginNativeCode: testCase.hasPluginNativeCode });
418+
419+
await previewAppPluginsService.comparePluginsOnDevice(createPreviewLiveSyncData({ bundle: true }), device);
420+
421+
assert.equal(warnParams.length, testCase.expectedWarnings.length);
422+
testCase.expectedWarnings.forEach(warning => assert.include(warnParams, warning));
423+
});
424+
});
425+
});
235426
describe("getExternalPlugins", () => {
236427
const testCases = [
237428
{

0 commit comments

Comments
 (0)