Skip to content

chore: don't show warnings for {N} plugins without native code when bundle option is provided #3904

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 2 commits into from
Sep 18, 2018
Merged

chore: don't show warnings for {N} plugins without native code when bundle option is provided #3904

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b65d78
chore: don't show warnings for {N} plugins without native code when …
Fatme Sep 18, 2018
a2952bc
chore: fix PR comments
Fatme Sep 18, 2018
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/preview-app-livesync.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ declare global {
}

interface IPreviewAppPluginsService {
comparePluginsOnDevice(device: Device): Promise<void>;
comparePluginsOnDevice(data: IPreviewAppLiveSyncData, device: Device): Promise<void>;
getExternalPlugins(device: Device): string[];
}

Expand Down
4 changes: 2 additions & 2 deletions lib/services/livesync/playground/preview-app-livesync-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class PreviewAppLiveSyncService implements IPreviewAppLiveSyncService {
startSyncFilesTimeout: startSyncFilesTimeout.bind(this)
}
});
await this.$previewAppPluginsService.comparePluginsOnDevice(device);
await this.$previewAppPluginsService.comparePluginsOnDevice(data, device);
const payloads = await this.syncFilesForPlatformSafe(data, device.platform);

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

for (const device of this.$previewSdkService.connectedDevices) {
await this.$previewAppPluginsService.comparePluginsOnDevice(device);
await this.$previewAppPluginsService.comparePluginsOnDevice(data, device);
}

const platforms = _(this.$previewSdkService.connectedDevices)
Expand Down
30 changes: 27 additions & 3 deletions lib/services/livesync/playground/preview-app-plugins-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as semver from "semver";
import * as util from "util";
import { Device } from "nativescript-preview-sdk";
import { PluginComparisonMessages } from "./preview-app-constants";
import { NODE_MODULES_DIR_NAME } from "../../../common/constants";

export class PreviewAppPluginsService implements IPreviewAppPluginsService {
private previewAppVersionWarnings: IDictionary<string[]> = {};
Expand All @@ -11,15 +12,15 @@ export class PreviewAppPluginsService implements IPreviewAppPluginsService {
private $logger: ILogger,
private $projectData: IProjectData) { }

public async comparePluginsOnDevice(device: Device): Promise<void> {
public async comparePluginsOnDevice(data: IPreviewAppLiveSyncData, device: Device): Promise<void> {
if (!this.previewAppVersionWarnings[device.previewAppVersion]) {
const devicePlugins = this.getDevicePlugins(device);
const localPlugins = this.getLocalPlugins();
const warnings = _.keys(localPlugins)
.map(localPlugin => {
const localPluginVersion = localPlugins[localPlugin];
const devicePluginVersion = devicePlugins[localPlugin];
return this.getWarningForPlugin(localPlugin, localPluginVersion, devicePluginVersion, device.id);
return this.getWarningForPlugin(data, localPlugin, localPluginVersion, devicePluginVersion, device);
})
.filter(item => !!item);
this.previewAppVersionWarnings[device.previewAppVersion] = warnings;
Expand Down Expand Up @@ -62,7 +63,15 @@ export class PreviewAppPluginsService implements IPreviewAppPluginsService {
}
}

private getWarningForPlugin(localPlugin: string, localPluginVersion: string, devicePluginVersion: string, deviceId: string): string {
private getWarningForPlugin(data: IPreviewAppLiveSyncData, localPlugin: string, localPluginVersion: string, devicePluginVersion: string, device: Device): string {
if (data && data.appFilesUpdaterOptions && data.appFilesUpdaterOptions.bundle && this.isNativeScriptPluginWithoutNativeCode(localPlugin, device.platform)) {
return null;
}

return this.getWarningForPluginCore(localPlugin, localPluginVersion, devicePluginVersion, device.id);
}

private getWarningForPluginCore(localPlugin: string, localPluginVersion: string, devicePluginVersion: string, deviceId: string): string {
this.$logger.trace(`Comparing plugin ${localPlugin} with localPluginVersion ${localPluginVersion} and devicePluginVersion ${devicePluginVersion}`);

if (devicePluginVersion) {
Expand All @@ -80,5 +89,20 @@ export class PreviewAppPluginsService implements IPreviewAppPluginsService {

return util.format(PluginComparisonMessages.PLUGIN_NOT_INCLUDED_IN_PREVIEW_APP, localPlugin, deviceId);
}

private isNativeScriptPluginWithoutNativeCode(localPlugin: string, platform: string): boolean {
return this.isNativeScriptPlugin(localPlugin) && !this.hasNativeCode(localPlugin, platform);
}

private isNativeScriptPlugin(localPlugin: string): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can move this method to pluginsService.

const pluginPackageJsonPath = path.join(this.$projectData.projectDir, NODE_MODULES_DIR_NAME, localPlugin, "package.json");
const pluginPackageJsonContent = this.$fs.readJson(pluginPackageJsonPath);
return pluginPackageJsonContent && pluginPackageJsonContent.nativescript;
}

private hasNativeCode(localPlugin: string, platform: string): boolean {
const nativeFolderPath = path.join(this.$projectData.projectDir, NODE_MODULES_DIR_NAME, localPlugin, "platforms", platform.toLowerCase());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"platforms" -> PLATFORMS_DIR_NAME from constants

return this.$fs.exists(nativeFolderPath) && !this.$fs.isEmptyDir(nativeFolderPath);
}
}
$injector.register("previewAppPluginsService", PreviewAppPluginsService);
208 changes: 197 additions & 11 deletions test/services/playground/preview-app-plugins-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,43 @@ import { PluginComparisonMessages } from "../../../lib/services/livesync/playgro
let readJsonParams: string[] = [];
let warnParams: string[] = [];

function createTestInjector(localPlugins: IStringDictionary): IInjector {
const deviceId = "myTestDeviceId";
const projectDir = "testProjectDir";

function createTestInjector(localPlugins: IStringDictionary, options?: { isNativeScriptPlugin?: boolean, hasPluginNativeCode?: boolean }): IInjector {
options = options || {};
const injector = new Yok();
injector.register("fs", {
readJson: (filePath: string) => {
readJsonParams.push(filePath);
return {
const result: any = {
dependencies: localPlugins
};

if (options.isNativeScriptPlugin) {
result.nativescript = {};
}

return result;
},
exists: (filePath: string) => {
return options.hasPluginNativeCode;
},
isEmptyDir: (filePath: string) => {
return !options.hasPluginNativeCode;
}
});
injector.register("logger", {
trace: () => ({}),
warn: (message: string) => warnParams.push(message)
});
injector.register("projectData", {
projectDir: "testProjectDir"
projectDir
});
injector.register("previewAppPluginsService", PreviewAppPluginsService);
return injector;
}

const deviceId = "myTestDeviceId";

function createDevice(plugins: string): Device {
return {
id: deviceId,
Expand All @@ -45,8 +59,20 @@ function createDevice(plugins: string): Device {
};
}

function setup(localPlugins: IStringDictionary, previewAppPlugins: IStringDictionary): any {
const injector = createTestInjector(localPlugins);
function createPreviewLiveSyncData(options?: { bundle: boolean }) {
return {
projectDir,
appFilesUpdaterOptions: {
release: false,
bundle: options.bundle
},
env: {}
};
}

function setup(localPlugins: IStringDictionary, previewAppPlugins: IStringDictionary,
options?: { isNativeScriptPlugin: boolean, hasPluginNativeCode: boolean }): any {
const injector = createTestInjector(localPlugins, options);
const previewAppPluginsService = injector.resolve("previewAppPluginsService");
const device = createDevice(JSON.stringify(previewAppPlugins));

Expand All @@ -57,7 +83,7 @@ function setup(localPlugins: IStringDictionary, previewAppPlugins: IStringDictio
}

describe("previewAppPluginsService", () => {
describe("comparePluginsOnDevice", () => {
describe("comparePluginsOnDevice without bundle", () => {
it("should persist warnings per preview app's version", async () => {
const localPlugins = {
"nativescript-facebook": "2.2.3",
Expand All @@ -84,7 +110,9 @@ describe("previewAppPluginsService", () => {
return originalGetLocalPlugins.apply(previewAppPluginsService);
};

await previewAppPluginsService.comparePluginsOnDevice(createDevice(JSON.stringify(previewAppPlugins)));
const previewLiveSyncData = createPreviewLiveSyncData({ bundle: false });

await previewAppPluginsService.comparePluginsOnDevice(previewLiveSyncData, createDevice(JSON.stringify(previewAppPlugins)));

const expectedWarnings = [
util.format(PluginComparisonMessages.PLUGIN_NOT_INCLUDED_IN_PREVIEW_APP, "nativescript-facebook", deviceId),
Expand All @@ -98,7 +126,7 @@ describe("previewAppPluginsService", () => {
isGetLocalPluginsCalled = false;
warnParams = [];

await previewAppPluginsService.comparePluginsOnDevice(createDevice(JSON.stringify(previewAppPlugins)));
await previewAppPluginsService.comparePluginsOnDevice(previewLiveSyncData, createDevice(JSON.stringify(previewAppPlugins)));

assert.isFalse(isGetDevicePluginsCalled);
assert.isFalse(isGetLocalPluginsCalled);
Expand Down Expand Up @@ -225,13 +253,171 @@ describe("previewAppPluginsService", () => {
it(`${testCase.name}`, async () => {
const { previewAppPluginsService, device } = setup(testCase.localPlugins, testCase.previewAppPlugins);

await previewAppPluginsService.comparePluginsOnDevice(device);
await previewAppPluginsService.comparePluginsOnDevice(createPreviewLiveSyncData({ bundle: false }), device);

assert.equal(warnParams.length, testCase.expectedWarnings.length);
testCase.expectedWarnings.forEach(warning => assert.include(warnParams, warning));
});
}
});
describe("comparePluginsOnDevice with bundle", () => {
const testCases = [
{
name: "should show warning for non nativescript plugin that has lower major version",
localPlugins: {
lodash: "1.2.3"
},
previewAppPlugins: {
lodash: "2.3.3"
},
isNativeScriptPlugin: false,
hasPluginNativeCode: false,
expectedWarnings: [
util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "lodash", "1.2.3", "2.3.3")
]
},
{
name: "should show warning for non nativescript plugin that has greather major version",
localPlugins: {
lodash: "3.2.3"
},
previewAppPlugins: {
lodash: "2.3.3"
},
isNativeScriptPlugin: false,
hasPluginNativeCode: false,
expectedWarnings: [
util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "lodash", "3.2.3", "2.3.3")
]
},
{
name: "should show warning for non nativescript plugin that has greather minor version",
localPlugins: {
lodash: "3.4.5"
},
previewAppPlugins: {
lodash: "3.3.0"
},
isNativeScriptPlugin: false,
hasPluginNativeCode: false,
expectedWarnings: [
util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_GREATHER_MINOR_VERSION, "lodash", "3.4.5", "3.3.0")
]
},
{
name: "should not show warning for non nativescript plugin that has the same version",
localPlugins: {
lodash: "3.4.5"
},
previewAppPlugins: {
lodash: "3.4.5"
},
isNativeScriptPlugin: false,
hasPluginNativeCode: false,
expectedWarnings: []
},
{
name: "should not show warning for nativescript plugin without native code that has lower major version",
localPlugins: {
"nativescript-theme-core": "2.4.5"
},
previewAppPlugins: {
"nativescript-theme-core": "3.4.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: false,
expectedWarnings: <string[]>[]
},
{
name: "should not show warning for nativescript plugin without native code that has greather major version",
localPlugins: {
"nativescript-theme-core": "4.4.5"
},
previewAppPlugins: {
"nativescript-theme-core": "3.4.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: false,
expectedWarnings: []
},
{
name: "should not show warning for nativescript plugin without native code that has greather minor version",
localPlugins: {
"nativescript-theme-core": "4.6.5"
},
previewAppPlugins: {
"nativescript-theme-core": "4.4.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: false,
expectedWarnings: []
},
{
name: "should not show warning for nativescript plugin without native code that has the same version",
localPlugins: {
"nativescript-theme-core": "4.6.5"
},
previewAppPlugins: {
"nativescript-theme-core": "4.6.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: false,
expectedWarnings: []
},
{
name: "should show warning for nativescript plugin with native code that has lower major version",
localPlugins: {
"nativescript-theme-core": "2.4.5"
},
previewAppPlugins: {
"nativescript-theme-core": "3.4.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: true,
expectedWarnings: [util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "nativescript-theme-core", "2.4.5", "3.4.5")]
},
{
name: "should show warning for nativescript plugin with native code that has greather major version",
localPlugins: {
"nativescript-theme-core": "4.4.5"
},
previewAppPlugins: {
"nativescript-theme-core": "3.4.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: true,
expectedWarnings: [util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_DIFFERENCE_IN_MAJOR_VERSION, "nativescript-theme-core", "4.4.5", "3.4.5")]
},
{
name: "should show warning for nativescript plugin with native code that has greather minor version",
localPlugins: {
"nativescript-theme-core": "4.4.5"
},
previewAppPlugins: {
"nativescript-theme-core": "4.3.5"
},
isNativeScriptPlugin: true,
hasPluginNativeCode: true,
expectedWarnings: [util.format(PluginComparisonMessages.LOCAL_PLUGIN_WITH_GREATHER_MINOR_VERSION, "nativescript-theme-core", "4.4.5", "4.3.5")]
},
];

afterEach(() => {
warnParams = [];
readJsonParams = [];
});

_.each(testCases, testCase => {
it(`${testCase.name}`, async () => {
const { previewAppPluginsService, device } = setup(testCase.localPlugins, testCase.previewAppPlugins, { isNativeScriptPlugin: testCase.isNativeScriptPlugin, hasPluginNativeCode: testCase.hasPluginNativeCode });

await previewAppPluginsService.comparePluginsOnDevice(createPreviewLiveSyncData({ bundle: true }), device);

assert.equal(warnParams.length, testCase.expectedWarnings.length);
testCase.expectedWarnings.forEach(warning => assert.include(warnParams, warning));
});
});
});
describe("getExternalPlugins", () => {
const testCases = [
{
Expand Down