Add JIT support for AngularCompilerPlugin by filipesilva · Pull Request #7684 · angular/angular-cli

filipesilva · 2017-09-13T16:18:02Z

Fix #2034
Fix #7623

filipesilva · 2017-09-13T16:18:18Z

Followup to #7512

hansl · 2017-09-14T15:10:51Z

packages/@ngtools/webpack/src/angular_compiler_plugin.ts

  private _tsFilenames: string[];
  // Should be Program from @angular/compiler-cli instead of any.
-  private _program: any;
+  private _program: ts.Program & any;


I'm not sure & any is even something... Wouldn't that be any? Any reason you cannot put ts.Program there?

When the interface is available in a stable manner, we should be doing ts.Program & Program. Meanwhile I wanted to leave a marker there that indicated it's not just ts.Program.

For some of these we should be able to not have the & any but for others that won't be possible since methods/props are being used that don't exist on ts.Program.

hansl · 2017-09-14T15:11:57Z

packages/@ngtools/webpack/src/angular_compiler_plugin.ts

+        let modulePath: string, moduleKey: string;

-        if (factoryKey in this._lazyRoutes) {
-          if (factoryPath === null) {


You removed this part, which removed lazy routes that were deleted during a rebuild. Did you want to keep a check to remove lazy routes ?

I checked earlier today and this actually doesn't do anything, as far as I could tell. When a lazy route goes away the discovered routes for this module is an empty object, so there's nothing to remove. There's a TODO just above to actually add this functionality.

hansl · 2017-09-14T15:12:59Z

packages/@ngtools/webpack/src/angular_compiler_plugin.ts

  // and uses CustomTransformers.
  // Should be Program and CustomTransformers from @angular/compiler-cli instead of any.
-  private _emit(program: any, customTransformers: any) {
+  private _emit(program: ts.Program & any, customTransformers: ts.CustomTransformers & any) {


It seems to me those & any are wrong. You probably want a declare type Program = ... and declare type CustomTransformers = ... at the top of the file to remove those weird typings in arguments.

hansl · 2017-09-14T15:16:25Z

packages/@ngtools/webpack/src/angular_compiler_plugin.ts

  private _basePath: string;
  private _transformMap: Map<string, TransformOperation[]> = new Map();
  private _platform: PLATFORM;
+  private _JITMode = false;


We capitalize acronyms and initialisms as if they were words. _JitMode.

googlebot · 2017-09-15T13:01:13Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

googlebot · 2017-09-15T13:13:35Z

CLAs look good, thanks!

filipesilva · 2017-09-15T14:55:47Z

Blocked by #7702.

ocombe · 2017-09-22T16:07:15Z

packages/@ngtools/webpack/src/transformers/register_locale_data.spec.ts

+      `;
+      const output = stripIndent`
+        import __locale_fr__ from "@angular/common/locales/fr";
+        import { registerLocaleData as registerLocaleData } from "@angular/common";


should be import { registerLocaleData } from "@angular/common";, I probably did something wrong in my transformer (not really wrong but useless)

…ormer

angular-automatic-lock-bot · 2019-09-12T04:42:21Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

filipesilva requested a review from hansl as a code owner September 13, 2017 16:18

googlebot added the cla: yes label Sep 13, 2017

filipesilva force-pushed the angular-compiler-jit branch 7 times, most recently from fbb49b6 to 9ce70c4 Compare September 14, 2017 13:24

hansl suggested changes Sep 14, 2017

View reviewed changes

filipesilva force-pushed the angular-compiler-jit branch 4 times, most recently from 106ddb6 to 67077c3 Compare September 15, 2017 13:01

googlebot added cla: no and removed cla: yes labels Sep 15, 2017

filipesilva force-pushed the angular-compiler-jit branch from 67077c3 to 36bd774 Compare September 15, 2017 13:13

googlebot added cla: yes and removed cla: no labels Sep 15, 2017

filipesilva force-pushed the angular-compiler-jit branch from 36bd774 to 094bc3d Compare September 15, 2017 14:55

filipesilva force-pushed the angular-compiler-jit branch 6 times, most recently from a69dda5 to 3ee989d Compare September 21, 2017 13:47

filipesilva force-pushed the angular-compiler-jit branch 3 times, most recently from e7c1ec7 to f822fd4 Compare September 22, 2017 16:03

ocombe reviewed Sep 22, 2017

View reviewed changes

filipesilva force-pushed the angular-compiler-jit branch 2 times, most recently from 0934f13 to 657beef Compare September 22, 2017 16:26

filipesilva self-assigned this Sep 25, 2017

filipesilva force-pushed the angular-compiler-jit branch from 657beef to 2b3f311 Compare September 26, 2017 16:46

This was referenced Sep 27, 2017

refactor(@ngtools/webpack): use emit when available #7285

Closed

refactor(@ngtools/webpack): use ts transforms in refactor #7134

Closed

filipesilva force-pushed the angular-compiler-jit branch from 2b3f311 to 662709a Compare September 27, 2017 11:37

filipesilva changed the title ~~Angular compiler jit~~ Add JIT support for AngularCompilerPlugin Sep 27, 2017

filipesilva force-pushed the angular-compiler-jit branch 3 times, most recently from 2b862aa to 97ce26b Compare September 28, 2017 12:59

hansl previously approved these changes Sep 28, 2017

View reviewed changes

filipesilva added 3 commits September 28, 2017 19:36

feat(@ngtools/webpack): add JIT support to AngularCompilerPlugin

c6c6906

test(@ngtools/webpack): add transformer tests

3bd1547

refactor(@ngtools/webpack): add internal only benchmark timings

8133f7a

filipesilva dismissed hansl’s stale review via 2c2f3d2 September 28, 2017 18:36

filipesilva force-pushed the angular-compiler-jit branch from 97ce26b to 2c2f3d2 Compare September 28, 2017 18:36

filipesilva added 3 commits September 28, 2017 19:38

feat(@ngtools/webpack): use forked type checker

a052d2f

test(@ngtools/webpack): add unit test for register_locate_data transf…

251efb9

…ormer

feat(@angular/cli): support xi18n with Angular 5

6c4d309

filipesilva force-pushed the angular-compiler-jit branch from 2c2f3d2 to 6c4d309 Compare September 28, 2017 18:39

hansl approved these changes Sep 28, 2017

View reviewed changes

filipesilva merged commit 335bd04 into angular:master Sep 28, 2017

filipesilva deleted the angular-compiler-jit branch September 28, 2017 20:12

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019

Conversation

filipesilva commented Sep 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

filipesilva commented Sep 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

googlebot commented Sep 15, 2017

Uh oh!

googlebot commented Sep 15, 2017

Uh oh!

filipesilva commented Sep 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

filipesilva commented Sep 13, 2017 •

edited

Loading