From 12db2856628d3fcabb725dfc876c4bbac93a4037 Mon Sep 17 00:00:00 2001 From: fatme Date: Fri, 1 Mar 2019 07:04:34 +0200 Subject: [PATCH 1/4] feat: enable hmr by default for new projects --- lib/common/services/commands-service.ts | 5 ++++- lib/declarations.d.ts | 1 + lib/definitions/project.d.ts | 6 ++++++ lib/options.ts | 20 ++++++++++++++++++++ lib/project-data.ts | 2 ++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index 8cb47a8e9f..4a6cc474e4 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -27,7 +27,10 @@ export class CommandsService implements ICommandsService { private $staticConfig: Config.IStaticConfig, private $helpService: IHelpService, private $extensibilityService: IExtensibilityService, - private $optionsTracker: IOptionsTracker) { + private $optionsTracker: IOptionsTracker, + private $projectDataService: IProjectDataService) { + const projectData = this.$projectDataService.getProjectData(); + this.$options.setupOptions(projectData); } public allCommands(opts: { includeDevCommands: boolean }): string[] { diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index 0b20e2139f..e7c4a4456d 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -568,6 +568,7 @@ interface IOptions extends IRelease, IDeviceIdentifier, IJustLaunch, IAvd, IAvai link: boolean; analyticsLogFile: string; performance: Object; + setupOptions(projectData: IProjectData): void; } interface IEnvOptions { diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index 4af5382abc..b0cb22b7cc 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -74,6 +74,7 @@ interface INsConfig { appPath?: string; appResourcesPath?: string; shared?: boolean; + isHmrEnabledByDefault?: boolean; } interface IProjectData extends ICreateProjectData { @@ -99,6 +100,11 @@ interface IProjectData extends ICreateProjectData { */ isShared: boolean; + /** + * Defines if the project has hmr enabled by default + */ + isHmrEnabledByDefault: boolean; + /** * Initializes project data with the given project directory. If none supplied defaults to --path option or cwd. * @param {string} projectDir Project root directory. diff --git a/lib/options.ts b/lib/options.ts index 9c1000f297..efa8eb09d6 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -21,6 +21,26 @@ export class Options { public options: IDictionary; + public setupOptions(projectData: IProjectData): void { + if (this.argv.release && this.argv.hmr) { + this.$errors.failWithoutHelp("The options --release and --hmr cannot be used simultaneously."); + } + + // HACK: temporary solution for 5.3.0 release (until the webpack only feature) + const parsed = require("yargs-parser")(process.argv.slice(2), { 'boolean-negation': false }); + // --no-hmr -> hmr: false or --hmr false -> hmr: 'false' + const noHmr = parsed && (parsed.hmr === false || parsed.hmr === 'false'); + if (noHmr) { + this.argv.hmr = false; + return; + } + + if (projectData.isHmrEnabledByDefault) { + this.argv.bundle = this.argv.bundle !== undefined ? this.argv.bundle : "webpack"; + this.argv.hmr = !this.argv.release; + } + } + constructor(private $errors: IErrors, private $staticConfig: Config.IStaticConfig, private $settingsService: ISettingsService) { diff --git a/lib/project-data.ts b/lib/project-data.ts index 2fe31b0f2f..7aa3a16543 100644 --- a/lib/project-data.ts +++ b/lib/project-data.ts @@ -61,6 +61,7 @@ export class ProjectData implements IProjectData { public buildXcconfigPath: string; public podfilePath: string; public isShared: boolean; + public isHmrEnabledByDefault: boolean; constructor(private $fs: IFileSystem, private $errors: IErrors, @@ -135,6 +136,7 @@ export class ProjectData implements IProjectData { this.buildXcconfigPath = path.join(this.appResourcesDirectoryPath, this.$devicePlatformsConstants.iOS, constants.BUILD_XCCONFIG_FILE_NAME); this.podfilePath = path.join(this.appResourcesDirectoryPath, this.$devicePlatformsConstants.iOS, constants.PODFILE_NAME); this.isShared = !!(this.nsConfig && this.nsConfig.shared); + this.isHmrEnabledByDefault = !!(this.nsConfig && this.nsConfig.isHmrEnabledByDefault); return; } From 7d696023303e003fb5e0f0396b57d55056847444 Mon Sep 17 00:00:00 2001 From: fatme Date: Fri, 1 Mar 2019 07:06:23 +0200 Subject: [PATCH 2/4] chore: add unit tests --- test/options.ts | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ test/stubs.ts | 1 + 2 files changed, 93 insertions(+) diff --git a/test/options.ts b/test/options.ts index cfa225e2f7..b046ff32ab 100644 --- a/test/options.ts +++ b/test/options.ts @@ -261,6 +261,98 @@ describe("options", () => { }); }); + + describe("setupOptions", () => { + const testsWithNoHmrByDefault = [ + { + name: "no nsconfig.json ", + isHmrEnabledByDefault: false, + args: [], + expectedHmr: false, + expectedBundle: false + }, + { + name: "no nsconfig.json and --hmr is provided", + isHmrEnabledByDefault: false, + args: ["--hmr"], + expectedHmr: true, + expectedBundle: true + }, + { + name: "no nsconfig.json and --no-hmr is provided", + isHmrEnabledByDefault: false, + args: ["--no-hmr"], + expectedHmr: false, + expectedBundle: false + }, + { + name: "no nsconfig.json and --release is provided", + isHmrEnabledByDefault: false, + args: ["--release"], + expectedHmr: false, + expectedBundle: false + }, + { + name: "no nsconfig.json and --bundle is provided", + isHmrEnabledByDefault: false, + args: ["--bundle"], + expectedHmr: false, + expectedBundle: true + } + ]; + const testsWithHmrByDefault = [ + { + name: "has nsconfig.json", + isHmrEnabledByDefault: true, + expectedHmr: true, + expectedBundle: true + }, + { + name: "has nsconfig.json and --hmr is provided", + isHmrEnabledByDefault: true, + args: ["--hmr"], + expectedHmr: true, + expectedBundle: true + }, + { + name: "has nsconfig.json and --no-hmr is provided", + isHmrEnabledByDefault: true, + args: ["--no-hmr"], + expectedHmr: false, + expectedBundle: false + }, + { + name: "has nsconfig.json and --release is provided", + isHmrEnabledByDefault: true, + args: ["--release"], + expectedHmr: false, + expectedBundle: true + }, + { + name: "has nsconfig.json and --bundle is provided", + isHmrEnabledByDefault: true, + args: ["--bundle"], + expectedHmr: true, + expectedBundle: true + } + ]; + const testCases = testsWithNoHmrByDefault.concat(testsWithHmrByDefault); + + _.each(testCases, testCase => { + it(`should pass correctly when ${testCase.name}`, () => { + (testCase.args || []).forEach(arg => process.argv.push(arg)); + + const options = createOptions(testInjector); + const projectData = { isHmrEnabledByDefault: testCase.isHmrEnabledByDefault }; + options.setupOptions(projectData); + + (testCase.args || []).forEach(arg => process.argv.pop()); + + assert.equal(!!options.argv.hmr, !!testCase.expectedHmr); + assert.equal(!!options.argv.bundle, !!testCase.expectedBundle); + }); + }); + }); }); function createOptionsWithProfileDir(defaultProfileDir?: string): IOptions { diff --git a/test/stubs.ts b/test/stubs.ts index 45c04dd3c9..e016450432 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -331,6 +331,7 @@ export class ProjectDataStub implements IProjectData { public buildXcconfigPath: string; public podfilePath: string; public isShared: boolean; + public isHmrEnabledByDefault: boolean; public initializeProjectData(projectDir?: string): void { this.projectDir = this.projectDir || projectDir; From 08f2a8e90ee7a40bd98b4af064bfa67dca340278 Mon Sep 17 00:00:00 2001 From: fatme Date: Wed, 6 Mar 2019 08:07:36 +0200 Subject: [PATCH 3/4] chore: fix failing unit tests --- lib/common/services/commands-service.ts | 8 +++++++- lib/options.ts | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index 4a6cc474e4..d441a2959a 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -29,7 +29,13 @@ export class CommandsService implements ICommandsService { private $extensibilityService: IExtensibilityService, private $optionsTracker: IOptionsTracker, private $projectDataService: IProjectDataService) { - const projectData = this.$projectDataService.getProjectData(); + let projectData = null; + try { + projectData = this.$projectDataService.getProjectData(); + } catch (err) { + this.$logger.trace(`Error while trying to get project data. More info: ${err}`); + } + this.$options.setupOptions(projectData); } diff --git a/lib/options.ts b/lib/options.ts index efa8eb09d6..66bcf187a5 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -35,7 +35,7 @@ export class Options { return; } - if (projectData.isHmrEnabledByDefault) { + if (projectData && projectData.isHmrEnabledByDefault) { this.argv.bundle = this.argv.bundle !== undefined ? this.argv.bundle : "webpack"; this.argv.hmr = !this.argv.release; } From a952a47ab035aa3c4f9e2a456d820636f26db778 Mon Sep 17 00:00:00 2001 From: fatme Date: Sun, 10 Mar 2019 14:02:08 +0200 Subject: [PATCH 4/4] fix: change the logic so useLegacyWorkflow name is used from nsconfig file --- lib/definitions/project.d.ts | 4 +- lib/options.ts | 17 +++-- lib/project-data.ts | 4 +- test/options.ts | 135 +++++++++++++++++++---------------- test/stubs.ts | 2 +- 5 files changed, 93 insertions(+), 69 deletions(-) diff --git a/lib/definitions/project.d.ts b/lib/definitions/project.d.ts index b0cb22b7cc..693d433772 100644 --- a/lib/definitions/project.d.ts +++ b/lib/definitions/project.d.ts @@ -74,7 +74,7 @@ interface INsConfig { appPath?: string; appResourcesPath?: string; shared?: boolean; - isHmrEnabledByDefault?: boolean; + useLegacyWorkflow?: boolean; } interface IProjectData extends ICreateProjectData { @@ -103,7 +103,7 @@ interface IProjectData extends ICreateProjectData { /** * Defines if the project has hmr enabled by default */ - isHmrEnabledByDefault: boolean; + useLegacyWorkflow: boolean; /** * Initializes project data with the given project directory. If none supplied defaults to --path option or cwd. diff --git a/lib/options.ts b/lib/options.ts index 66bcf187a5..79931e43e7 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -28,16 +28,25 @@ export class Options { // HACK: temporary solution for 5.3.0 release (until the webpack only feature) const parsed = require("yargs-parser")(process.argv.slice(2), { 'boolean-negation': false }); + const noBundle = parsed && (parsed.bundle === false || parsed.bundle === 'false'); + if (noBundle && this.argv.hmr) { + this.$errors.failWithoutHelp("The options --no-bundle and --hmr cannot be used simultaneously."); + } + + if (projectData && projectData.useLegacyWorkflow === false) { + this.argv.bundle = this.argv.bundle !== undefined ? this.argv.bundle : "webpack"; + this.argv.hmr = !this.argv.release; + } + // --no-hmr -> hmr: false or --hmr false -> hmr: 'false' const noHmr = parsed && (parsed.hmr === false || parsed.hmr === 'false'); if (noHmr) { this.argv.hmr = false; - return; } - if (projectData && projectData.isHmrEnabledByDefault) { - this.argv.bundle = this.argv.bundle !== undefined ? this.argv.bundle : "webpack"; - this.argv.hmr = !this.argv.release; + if (noBundle) { + this.argv.bundle = undefined; + this.argv.hmr = false; } } diff --git a/lib/project-data.ts b/lib/project-data.ts index 7aa3a16543..a139e78eee 100644 --- a/lib/project-data.ts +++ b/lib/project-data.ts @@ -61,7 +61,7 @@ export class ProjectData implements IProjectData { public buildXcconfigPath: string; public podfilePath: string; public isShared: boolean; - public isHmrEnabledByDefault: boolean; + public useLegacyWorkflow: boolean; constructor(private $fs: IFileSystem, private $errors: IErrors, @@ -136,7 +136,7 @@ export class ProjectData implements IProjectData { this.buildXcconfigPath = path.join(this.appResourcesDirectoryPath, this.$devicePlatformsConstants.iOS, constants.BUILD_XCCONFIG_FILE_NAME); this.podfilePath = path.join(this.appResourcesDirectoryPath, this.$devicePlatformsConstants.iOS, constants.PODFILE_NAME); this.isShared = !!(this.nsConfig && this.nsConfig.shared); - this.isHmrEnabledByDefault = !!(this.nsConfig && this.nsConfig.isHmrEnabledByDefault); + this.useLegacyWorkflow = this.nsConfig && this.nsConfig.useLegacyWorkflow; return; } diff --git a/test/options.ts b/test/options.ts index b046ff32ab..1727991746 100644 --- a/test/options.ts +++ b/test/options.ts @@ -263,93 +263,108 @@ describe("options", () => { }); describe("setupOptions", () => { - const testsWithNoHmrByDefault = [ + const testCases = [ { - name: "no nsconfig.json ", - isHmrEnabledByDefault: false, + name: "no options are provided", args: [], - expectedHmr: false, - expectedBundle: false + data: [ + { useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: false }, + { useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true }, + { useLegacyWorkflow: true, expectedHmr: false, expectedBundle: false } + ] }, { - name: "no nsconfig.json and --hmr is provided", - isHmrEnabledByDefault: false, + name: " --hmr is provided", args: ["--hmr"], - expectedHmr: true, - expectedBundle: true + data: [ + { useLegacyWorkflow: undefined, expectedHmr: true, expectedBundle: true }, + { useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true }, + { useLegacyWorkflow: true, expectedHmr: true, expectedBundle: true } + ] }, { - name: "no nsconfig.json and --no-hmr is provided", - isHmrEnabledByDefault: false, + name: " --no-hmr is provided", args: ["--no-hmr"], - expectedHmr: false, - expectedBundle: false + data: [ + { useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: false }, + { useLegacyWorkflow: false, expectedHmr: false, expectedBundle: true }, + { useLegacyWorkflow: true, expectedHmr: false, expectedBundle: false } + ] }, { - name: "no nsconfig.json and --release is provided", - isHmrEnabledByDefault: false, - args: ["--release"], - expectedHmr: false, - expectedBundle: false - }, - { - name: "no nsconfig.json and --bundle is provided", - isHmrEnabledByDefault: false, + name: " --bundle is provided", args: ["--bundle"], - expectedHmr: false, - expectedBundle: true - } - ]; - const testsWithHmrByDefault = [ - { - name: "has nsconfig.json", - isHmrEnabledByDefault: true, - expectedHmr: true, - expectedBundle: true + data: [ + { useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: true }, + { useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true }, + { useLegacyWorkflow: true, expectedHmr: false, expectedBundle: true } + ] }, { - name: "has nsconfig.json and --hmr is provided", - isHmrEnabledByDefault: true, - args: ["--hmr"], - expectedHmr: true, - expectedBundle: true + name: " --no-bundle is provided", + args: ["--no-bundle"], + data: [ + { useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: false }, + { useLegacyWorkflow: false, expectedHmr: false, expectedBundle: false }, + { useLegacyWorkflow: true, expectedHmr: false, expectedBundle: false } + ] }, { - name: "has nsconfig.json and --no-hmr is provided", - isHmrEnabledByDefault: true, - args: ["--no-hmr"], - expectedHmr: false, - expectedBundle: false - }, - { - name: "has nsconfig.json and --release is provided", - isHmrEnabledByDefault: true, + name: " --release is provided", args: ["--release"], - expectedHmr: false, - expectedBundle: true + data: [ + { useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: false }, + { useLegacyWorkflow: false, expectedHmr: false, expectedBundle: true }, + { useLegacyWorkflow: true, expectedHmr: false, expectedBundle: false } + ] + } + ]; + + _.each([undefined, false, true], useLegacyWorkflow => { + _.each(testCases, testCase => { + it(`should pass correctly when ${testCase.name} and useLegacyWorkflow is ${useLegacyWorkflow}`, () => { + (testCase.args || []).forEach(arg => process.argv.push(arg)); + + const options = createOptions(testInjector); + const projectData = { useLegacyWorkflow }; + options.setupOptions(projectData); + + (testCase.args || []).forEach(arg => process.argv.pop()); + + const data = testCase.data.find(item => item.useLegacyWorkflow === useLegacyWorkflow); + + assert.equal(!!options.argv.hmr, !!data.expectedHmr); + assert.equal(!!options.argv.bundle, !!data.expectedBundle); + }); + }); + }); + + const testCasesExpectingToThrow = [ + { + name: "--release --hmr", + args: ["--release", "--hmr"], + expectedError: "The options --release and --hmr cannot be used simultaneously." }, { - name: "has nsconfig.json and --bundle is provided", - isHmrEnabledByDefault: true, - args: ["--bundle"], - expectedHmr: true, - expectedBundle: true + name: "--no-bundle --hmr", + args: ["--no-bundle", "--hmr"], + expectedError: "The options --no-bundle and --hmr cannot be used simultaneously." } ]; - const testCases = testsWithNoHmrByDefault.concat(testsWithHmrByDefault); - _.each(testCases, testCase => { - it(`should pass correctly when ${testCase.name}`, () => { + _.each(testCasesExpectingToThrow, testCase => { + it(`should fail when ${testCase.name}`, () => { + let actualError = null; + const errors = testInjector.resolve("errors"); + errors.failWithoutHelp = (error: string) => actualError = error; (testCase.args || []).forEach(arg => process.argv.push(arg)); const options = createOptions(testInjector); - const projectData = { isHmrEnabledByDefault: testCase.isHmrEnabledByDefault }; - options.setupOptions(projectData); + options.setupOptions(null); (testCase.args || []).forEach(arg => process.argv.pop()); - assert.equal(!!options.argv.hmr, !!testCase.expectedHmr); - assert.equal(!!options.argv.bundle, !!testCase.expectedBundle); + assert.deepEqual(actualError, testCase.expectedError); }); }); }); diff --git a/test/stubs.ts b/test/stubs.ts index e016450432..c98362dda3 100644 --- a/test/stubs.ts +++ b/test/stubs.ts @@ -331,7 +331,7 @@ export class ProjectDataStub implements IProjectData { public buildXcconfigPath: string; public podfilePath: string; public isShared: boolean; - public isHmrEnabledByDefault: boolean; + public useLegacyWorkflow: boolean; public initializeProjectData(projectDir?: string): void { this.projectDir = this.projectDir || projectDir;