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.

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

fix(@schematics/angular): fix syntax error in main.ts after generating universal #13396

Merged
merged 1 commit into from Jan 11, 2019

Conversation

Projects
None yet
5 participants
@mbenzenhoefer
Copy link
Contributor

mbenzenhoefer commented Jan 9, 2019

Add missing } to fix syntax error in main.ts file.

Fixes #13392

@mgechev

This comment has been minimized.

Copy link
Member

mgechev commented Jan 9, 2019

@mbenzenhoefer it'll be best if you add an e2e test to verify that the generated file is now valid.

@alan-agius4 @clydin the e2e failure doesn't seem related.

@alan-agius4

This comment has been minimized.

Copy link
Collaborator

alan-agius4 commented Jan 10, 2019

@mbenzenhoefer, thanks for this.

I think instead of an E2E, you can change the unit tests

it('should wrap the bootstrap call in a DOMContentLoaded event handler', () => {
const tree = schematicRunner.runSchematic('universal', defaultOptions, appTree);
const filePath = '/projects/bar/src/main.ts';
const contents = tree.readContent(filePath);
expect(contents).toMatch(/document.addEventListener\('DOMContentLoaded', \(\) => {/);
});
it('should wrap the bootstrap decleration in a DOMContentLoaded event handler', () => {
const filePath = '/projects/bar/src/main.ts';
appTree.overwrite(
filePath,
`
import { enableProdMode } from '@angular/core';
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
import { AppModule } from './app/app.module';
import { environment } from './environments/environment';
import { hmrBootstrap } from './hmr';
if (environment.production) {
enableProdMode();
}
const bootstrap = () => platformBrowserDynamic().bootstrapModule(AppModule);
if (!hmrBootstrap) {
bootstrap().catch(err => console.log(err));
}
`,
);
const tree = schematicRunner.runSchematic('universal', defaultOptions, appTree);
const contents = tree.readContent(filePath);
expect(contents).toMatch(
/document.addEventListener\('DOMContentLoaded', \(\) => {[\n\r\s]+bootstrap\(\)/,
);
});

to something like:

it('should retain used aliased imports', () => {
const input = tags.stripIndent`
import { promise as fromPromise } from './const';
const used = fromPromise;
${dummyNode}
`;
const output = tags.stripIndent`
import { promise as fromPromise } from './const';
const used = fromPromise;
`;
const { program, compilerHost } = createTypescriptContext(input, additionalFiles);
const result = transformTypescript(undefined, [transformer(program)], program, compilerHost);
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);

To match the entire output

@mbenzenhoefer

This comment has been minimized.

Copy link
Contributor

mbenzenhoefer commented Jan 10, 2019

@alan-agius4 I was also thinking about that. Will adjust the test.

@mbenzenhoefer

This comment has been minimized.

Copy link
Contributor

mbenzenhoefer commented Jan 10, 2019

Changed the test to check for the valid syntax in RegExp. What do you think about this RegExp?
For me it was important to make sure that after the line ending with ; a new line with curly brace follows.

@alan-agius4 alan-agius4 removed the request for review from clydin Jan 10, 2019

@alan-agius4
Copy link
Collaborator

alan-agius4 left a comment

Hi, can you kindly squash all your commits into a single one, which also included Fixes #13392 as part of the commit footer?

Thanks.

fix(@schematics/angular): fix syntax error in main.ts after generatin…
…g universal

Add missing curly brace for application boostrap wrapper and test
Fixes #13392

@mbenzenhoefer mbenzenhoefer force-pushed the mbenzenhoefer:feature/13392-universal-main-syntax branch from 73c22d7 to f29a3c8 Jan 11, 2019

@kyliau kyliau merged commit 5064012 into angular:master Jan 11, 2019

13 checks passed

ci/angular: merge status All checks passed!
ci/angular: size cli/new-production/test-project/polyfills.js increased by 3.51KB.
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build-bazel Your tests passed on CircleCI!
Details
ci/circleci: e2e-cli Your tests passed on CircleCI!
Details
ci/circleci: e2e-node-8 Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: test-large Your tests passed on CircleCI!
Details
ci/circleci: validate Your tests passed on CircleCI!
Details
cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment