Skip to content
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

feat: enable hmr by default for new projects #4411

Merged
merged 4 commits into from
Mar 13, 2019
Merged

feat: enable hmr by default for new projects #4411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12db285
feat: enable hmr by default for new projects
Fatme Mar 1, 2019
7d69602
chore: add unit tests
Fatme Mar 1, 2019
08f2a8e
chore: fix failing unit tests
Fatme Mar 6, 2019
a952a47
fix: change the logic so useLegacyWorkflow name is used from nsconfig…
Fatme Mar 10, 2019
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
11 changes: 10 additions & 1 deletion lib/common/services/commands-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,16 @@ export class CommandsService implements ICommandsService {
private $staticConfig: Config.IStaticConfig,
private $helpService: IHelpService,
private $extensibilityService: IExtensibilityService,
private $optionsTracker: IOptionsTracker) {
private $optionsTracker: IOptionsTracker,
private $projectDataService: IProjectDataService) {
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);
}

public allCommands(opts: { includeDevCommands: boolean }): string[] {
Expand Down
1 change: 1 addition & 0 deletions lib/declarations.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ interface IOptions extends IRelease, IDeviceIdentifier, IJustLaunch, IAvd, IAvai
link: boolean;
analyticsLogFile: string;
performance: Object;
setupOptions(projectData: IProjectData): void;
}

interface IEnvOptions {
Expand Down
6 changes: 6 additions & 0 deletions lib/definitions/project.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ interface INsConfig {
appPath?: string;
appResourcesPath?: string;
shared?: boolean;
useLegacyWorkflow?: boolean;
}

interface IProjectData extends ICreateProjectData {
Expand All @@ -99,6 +100,11 @@ interface IProjectData extends ICreateProjectData {
*/
isShared: boolean;

/**
* Defines if the project has hmr enabled by default
*/
useLegacyWorkflow: boolean;

/**
* Initializes project data with the given project directory. If none supplied defaults to --path option or cwd.
* @param {string} projectDir Project root directory.
Expand Down
29 changes: 29 additions & 0 deletions lib/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,35 @@ export class Options {

public options: IDictionary<IDashedOption>;

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 });
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;
}

if (noBundle) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a warning for the user that we will override both hmr and bundle options.

this.argv.bundle = undefined;
this.argv.hmr = false;
}
}

constructor(private $errors: IErrors,
private $staticConfig: Config.IStaticConfig,
private $settingsService: ISettingsService) {
Expand Down
2 changes: 2 additions & 0 deletions lib/project-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export class ProjectData implements IProjectData {
public buildXcconfigPath: string;
public podfilePath: string;
public isShared: boolean;
public useLegacyWorkflow: boolean;

constructor(private $fs: IFileSystem,
private $errors: IErrors,
Expand Down Expand Up @@ -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.useLegacyWorkflow = this.nsConfig && this.nsConfig.useLegacyWorkflow;
return;
}

Expand Down
107 changes: 107 additions & 0 deletions test/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,113 @@ describe("options", () => {

});
});

describe("setupOptions", () => {
const testCases = [
{
name: "no options are provided",
args: [],
data: [
{ useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: false },
{ useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true },
{ useLegacyWorkflow: true, expectedHmr: false, expectedBundle: false }
]
},
{
name: " --hmr is provided",
args: ["--hmr"],
data: [
{ useLegacyWorkflow: undefined, expectedHmr: true, expectedBundle: true },
{ useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true },
{ useLegacyWorkflow: true, expectedHmr: true, expectedBundle: true }
]
},
{
name: " --no-hmr is provided",
args: ["--no-hmr"],
data: [
{ useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: false },
{ useLegacyWorkflow: false, expectedHmr: false, expectedBundle: true },
{ useLegacyWorkflow: true, expectedHmr: false, expectedBundle: false }
]
},
{
name: " --bundle is provided",
args: ["--bundle"],
data: [
{ useLegacyWorkflow: undefined, expectedHmr: false, expectedBundle: true },
{ useLegacyWorkflow: false, expectedHmr: true, expectedBundle: true },
{ useLegacyWorkflow: true, expectedHmr: false, 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: " --release is provided",
args: ["--release"],
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 = <IProjectData>{ 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: "--no-bundle --hmr",
args: ["--no-bundle", "--hmr"],
expectedError: "The options --no-bundle and --hmr cannot be used simultaneously."
}
];

_.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);
options.setupOptions(null);

(testCase.args || []).forEach(arg => process.argv.pop());

assert.deepEqual(actualError, testCase.expectedError);
});
});
});
});

function createOptionsWithProfileDir(defaultProfileDir?: string): IOptions {
Expand Down
1 change: 1 addition & 0 deletions test/stubs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ export class ProjectDataStub implements IProjectData {
public buildXcconfigPath: string;
public podfilePath: string;
public isShared: boolean;
public useLegacyWorkflow: boolean;

public initializeProjectData(projectDir?: string): void {
this.projectDir = this.projectDir || projectDir;
Expand Down