Ngcc es5 resolving by petebacondarwin · Pull Request #25060 · angular/angular

petebacondarwin · 2018-07-24T08:14:49Z

This will replace #25048 when #24897 has landed in master.
Ignoring the fixup commits at the end: the penultimate commit is from #25048 - and the final commit is the meat of this PR.

googlebot · 2018-07-24T08:15:03Z

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 or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok 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.

mary-poppins · 2018-07-24T08:24:43Z

You can preview 09d3f7c at https://pr25060-09d3f7c.ngbuilds.io/.

gkalpak · 2018-07-24T09:24:31Z

and to --> and

or and to --> to

gkalpak · 2018-07-24T09:37:02Z

I think it is also worth having tests like these (to test some aspects of looking for param initializers):

function baz() { var x; if (x === void 0) { x = 42; } return x; } var x; function qux() { if (x === void 0) { x = 42; } return x; }

Great tests you added there 👍
I still think the tests in my previous comment are worth adding too 😁

I thought I was in keeping with the sentiment of your tests!

The sentiment is great. It's just that your tests and my tests test different code paths.
(We need both.)

So many code paths!

mary-poppins · 2018-07-24T10:41:28Z

You can preview aa77e55 at https://pr25060-aa77e55.ngbuilds.io/.

mary-poppins · 2018-07-25T07:12:03Z

You can preview b6c8c85 at https://pr25060-b6c8c85.ngbuilds.io/.

mary-poppins · 2018-07-27T06:48:07Z

You can preview 9bb494b at https://pr25060-9bb494b.ngbuilds.io/.

mary-poppins · 2018-07-27T07:00:07Z

You can preview ce7a140 at https://pr25060-ce7a140.ngbuilds.io/.

mary-poppins · 2018-07-28T09:50:18Z

You can preview 5c46d8f at https://pr25060-5c46d8f.ngbuilds.io/.

gkalpak · 2018-07-28T09:58:27Z

Rebased on master (now that #24897 has been merged) and force-pushed.

trotyl · 2018-07-28T15:39:54Z

Sorry for an off-topic question, I wonder if current ngcc architecture will break use case of #24580 which doesn't use esm at all?

Previously compiler-cli needs only .metadata.json and .d.ts, rather than .js, making arbitrary JavaScript processing possible.

petebacondarwin · 2018-07-30T13:22:10Z

@trotyl thanks for linking this. I am pretty sure that Ngcc will not be able to consume the modified UMD file which is all that appears to be left after "securing" a project.

That being said, I believe that going forward, one would be able to republish the original secured files after they have been compiled with ngtsc

alxhub · 2018-08-07T13:14:48Z

+    let lookingForParamInitializers = true;
+
+    const statements = node.body && node.body.statements.filter(s => {
+      lookingForParamInitializers =


Hmm. I'm not a fan of the non-pure filter lambda. Can you implement this as a loop instead?

alxhub · 2018-08-07T17:48:54Z

@trotyl good point, and the answer as @petebacondarwin says is indeed no, it won't work with such packages. ngcc only operates on Angular Package Format packages.

trotyl · 2018-08-08T01:34:28Z

@alxhub @petebacondarwin Thanks for reply, I'm fine with that, if finally everything will be on ngtsc, it's reasonable for not wasting effort.

ngtsc's static resolver can evaluate function calls where parameters have default values. In TypeScript code these default values live on the function definition, but in ES5 code the default values are represented by statements in the function body. A new ReflectionHost method getDefinitionOfFunction() abstracts over this difference, and allows the static reflector to more accurately evaluate ES5 code.

…ection hosts

mary-poppins · 2018-08-08T09:04:09Z

You can preview e0e09ad at https://pr25060-e0e09ad.ngbuilds.io/.

petebacondarwin · 2018-08-21T22:01:57Z

Superceded by #25090

angular-automatic-lock-bot · 2019-09-13T23:52:11Z

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.}

petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release comp: ivy labels Jul 24, 2018

petebacondarwin added this to the Ivy milestone Jul 24, 2018

petebacondarwin requested review from alxhub and gkalpak July 24, 2018 08:14

googlebot added the cla: no label Jul 24, 2018

gkalpak approved these changes Jul 24, 2018

View reviewed changes

petebacondarwin force-pushed the ngcc-es5-resolving branch from aa77e55 to b6c8c85 Compare July 25, 2018 06:57

gkalpak mentioned this pull request Jul 25, 2018

feat(ivy): fix and enable esm2015/esm5 processing in ngcc #25090

Closed

3 tasks

petebacondarwin force-pushed the ngcc-es5-resolving branch from b6c8c85 to 9bb494b Compare July 27, 2018 06:35

petebacondarwin force-pushed the ngcc-es5-resolving branch from ce7a140 to 9bb494b Compare July 27, 2018 08:14

gkalpak force-pushed the ngcc-es5-resolving branch from 9bb494b to 5c46d8f Compare July 28, 2018 09:41

alxhub approved these changes Aug 7, 2018

View reviewed changes

alxhub and others added 2 commits August 8, 2018 08:02

feat(ivy): implement getDefinitionOfFunction on ES2015 and ES5 refl…

e0e09ad

…ection hosts

petebacondarwin force-pushed the ngcc-es5-resolving branch from 5c46d8f to e0e09ad Compare August 8, 2018 08:54

petebacondarwin removed the subtype: docs-deprecated label Aug 8, 2018

petebacondarwin added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 8, 2018

petebacondarwin closed this Aug 21, 2018

petebacondarwin deleted the ngcc-es5-resolving branch August 23, 2018 06:14

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

Conversation

petebacondarwin commented Jul 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Jul 24, 2018

Uh oh!

mary-poppins commented Jul 24, 2018

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Jul 24, 2018

Uh oh!

mary-poppins commented Jul 25, 2018

Uh oh!

mary-poppins commented Jul 27, 2018

Uh oh!

mary-poppins commented Jul 27, 2018

Uh oh!

mary-poppins commented Jul 28, 2018

Uh oh!

gkalpak commented Jul 28, 2018

Uh oh!

trotyl commented Jul 28, 2018

Uh oh!

petebacondarwin commented Jul 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alxhub commented Aug 7, 2018

Uh oh!

trotyl commented Aug 8, 2018

Uh oh!

mary-poppins commented Aug 8, 2018

Uh oh!

petebacondarwin commented Aug 21, 2018

Uh oh!

angular-automatic-lock-bot Bot commented Sep 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

petebacondarwin commented Jul 24, 2018 •

edited

Loading