feat: partial compilation of directives #39518

JoostK · 2020-10-30T22:56:51Z

See individual commits.

packages/compiler-cli/test/compliance/prelink/bootstrap.ts

petebacondarwin

Nice! I love how clean the whole PR feels. It is really easy to follow along with what it does.

packages/compiler-cli/src/ngtsc/annotations/src/directive.ts

petebacondarwin · 2020-11-02T12:44:50Z

packages/compiler/src/render3/partial/api.ts

+  // Version number of the metadata format. This is used to evolve the metadata
+  // interface later - the linker will be able to detect which version a library
+  // is using and interpret its metadata accordingly.
+  version: 1;


We talked about using the current framework version number here instead and then doing the checks via semantic version ranges at link time.

If we want to document different interfaces for different version ranges, then we could append something to the name of the interface... for example: R3DeclareDirectiveMetadata_from_11_1_0 or similar.

Yes, but doing that now would also require changes to the selector logic etc. which I felt was better left for another PR.

packages/compiler/src/render3/partial/api.ts

packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_directive_linker_1.ts

packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_linker_selector.ts

petebacondarwin · 2020-11-02T14:45:00Z

Also, I am not sure whether these commits should be feat or refactor... In any case, I think that the 3rd commit message should probably reworded slightly:

feat(compiler-cli): implement partial directive declaration linking 

This commit implements the logic to compile a partial declaration of a
directive into its full AOT compilation output.

petebacondarwin

LGTM!

petebacondarwin · 2020-11-02T20:53:21Z

packages/compiler/src/render3/partial/directive.ts

  meta.set(
      'predicate', Array.isArray(query.predicate) ? asLiteral(query.predicate) : query.predicate);
-  meta.set('descendants', o.literal(query.descendants));
+  if (query.descendants) {


It appears that descendants is true for ViewChild, ViewChildren and ContentChild. Only ContentChildren sets it to false. I am not sure which combination of these is the most common. Do you?
If it is not clear-cut, I guess it is simpler to go with the default being false. At least from a position of least surprise.

I would guess that ViewChild is most common, but honestly I have no idea what the distribution actually is.

I did consider basically this question but then for the first flag, which I suspect is more often true than false. But it did indeed feel simpler/more consistent to go with false as a default here.

jessicajaniuk

This looks really clean to me! LGTM!

petebacondarwin

Reviewed-for: public-api

… prelink target In PR angular#38938 an additional Bazel target was introduced for the compliance tests, as preparation to run the compliance tests in partial compilation mode and then apply the linker transform. The linker plugin itself was not available at the time but has since been implemented, so this commit updates the prelink target of the compliance tests to apply the linker transform using the Babel plugin. Actually emitting partial compilations to be transformed will be done in follow-up work.

… test's prelink target

This commit implements partial code generation for directives, which will be transformed by the linker plugin to fully AOT compiled code in follow-up work.

This commit implements the logic to compile a partial declaration of a directive into its full AOT compilation output.

…nking

…nent` to use `ɵɵ` prefix For consistency with other generated code, the partial declaration functions are renamed to use the `ɵɵ` prefix which indicates that it is generated API. This commit also removes the declaration from the public API golden file, as it's not yet considered stable at this point. Once the linker is finalized will these declaration function be included into the golden file.

AndrewKushnir · 2020-11-03T20:50:40Z

After changing $ng prefix to ɵɵng one it looks like public-api group approval is not needed for this PR.

packages/compiler/src/render3/partial/api.ts

goldens/public-api/core/core.d.ts

AndrewKushnir · 2020-11-04T02:18:38Z

Presubmit.

AndrewKushnir · 2020-11-04T17:30:38Z

FYI, presubmit is successful, I'm removing the "presubmit" label.

… prelink target (#39518) In PR #38938 an additional Bazel target was introduced for the compliance tests, as preparation to run the compliance tests in partial compilation mode and then apply the linker transform. The linker plugin itself was not available at the time but has since been implemented, so this commit updates the prelink target of the compliance tests to apply the linker transform using the Babel plugin. Actually emitting partial compilations to be transformed will be done in follow-up work. PR Close #39518

This commit implements partial code generation for directives, which will be transformed by the linker plugin to fully AOT compiled code in follow-up work. PR Close #39518

…39518) This commit implements the logic to compile a partial declaration of a directive into its full AOT compilation output. PR Close #39518

…nent` to use `ɵɵ` prefix (#39518) For consistency with other generated code, the partial declaration functions are renamed to use the `ɵɵ` prefix which indicates that it is generated API. This commit also removes the declaration from the public API golden file, as it's not yet considered stable at this point. Once the linker is finalized will these declaration function be included into the golden file. PR Close #39518

angular-automatic-lock-bot · 2020-12-05T16:30:13Z

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

JoostK added feature Issue that requests a new feature state: WIP area: compiler Issues related to `ngc`, Angular's template compiler target: minor This PR is targeted for the next minor release labels Oct 30, 2020

ngbot bot modified the milestone: needsTriage Oct 30, 2020

google-cla bot added the cla: yes label Oct 30, 2020

JoostK force-pushed the linker/directives branch from 0080f3d to 6911025 Compare October 30, 2020 23:16

JoostK changed the title ~~feat: linker compilation of directives~~ feat: partial compilation of directives Oct 30, 2020

JoostK marked this pull request as ready for review November 2, 2020 12:00

JoostK added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Nov 2, 2020

petebacondarwin reviewed Nov 2, 2020

View reviewed changes

packages/compiler-cli/test/compliance/prelink/bootstrap.ts Outdated Show resolved Hide resolved

petebacondarwin self-requested a review November 2, 2020 12:19

petebacondarwin requested changes Nov 2, 2020

View reviewed changes

JoostK force-pushed the linker/directives branch from 21ac625 to 292a0a6 Compare November 2, 2020 20:20

pullapprove bot requested a review from jessicajaniuk November 2, 2020 20:20

JoostK added this to In progress in ng-linker via automation Nov 2, 2020

JoostK moved this from In progress to Review in progress in ng-linker Nov 2, 2020

petebacondarwin approved these changes Nov 2, 2020

View reviewed changes

petebacondarwin added the action: presubmit The PR is in need of a google3 presubmit label Nov 2, 2020

jessicajaniuk approved these changes Nov 2, 2020

View reviewed changes

pullapprove bot requested review from atscott, IgorMinar, jelbourn and petebacondarwin November 2, 2020 23:34

petebacondarwin approved these changes Nov 3, 2020

View reviewed changes

JoostK added 10 commits November 3, 2020 21:34

fixup! refactor(compiler-cli): extend AST object and value helpers

5a2d7a2

fixup! test(compiler-cli): run the linker transform in the compliance…

b94f30b

… test's prelink target

feat(compiler-cli): partial compilation of directives

d57baea

This commit implements partial code generation for directives, which will be transformed by the linker plugin to fully AOT compiled code in follow-up work.

fixup! feat(compiler-cli): partial compilation of directives

a4a0b78

fixup! feat(compiler-cli): partial compilation of directives

b65b0ec

fixup! feat(compiler-cli): partial compilation of directives

f721ccb

feat(compiler-cli): implement partial directive declaration linking

05988c9

This commit implements the logic to compile a partial declaration of a directive into its full AOT compilation output.

fixup! feat(compiler-cli): implement partial directive declaration li…

02709f5

…nking

JoostK force-pushed the linker/directives branch from 292a0a6 to c04f9e3 Compare November 3, 2020 20:34

JoostK removed request for IgorMinar, atscott, jelbourn and alxhub November 3, 2020 20:41

JoostK added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 3, 2020

alxhub approved these changes Nov 3, 2020

View reviewed changes

packages/compiler/src/render3/partial/api.ts Outdated Show resolved Hide resolved

goldens/public-api/core/core.d.ts Outdated Show resolved Hide resolved

AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Nov 4, 2020

josephperrott closed this in 4eda87c Nov 4, 2020

ng-linker automation moved this from Review in progress to Done Nov 4, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: partial compilation of directives #39518

feat: partial compilation of directives #39518

JoostK commented Oct 30, 2020

petebacondarwin left a comment

petebacondarwin Nov 2, 2020

petebacondarwin Nov 2, 2020

JoostK Nov 2, 2020

petebacondarwin Nov 2, 2020

petebacondarwin commented Nov 2, 2020

petebacondarwin left a comment

petebacondarwin Nov 2, 2020

JoostK Nov 2, 2020

jessicajaniuk left a comment

petebacondarwin left a comment

AndrewKushnir commented Nov 3, 2020 •

edited

AndrewKushnir commented Nov 4, 2020

AndrewKushnir commented Nov 4, 2020

angular-automatic-lock-bot bot commented Dec 5, 2020

feat: partial compilation of directives #39518

feat: partial compilation of directives #39518

Conversation

JoostK commented Oct 30, 2020

petebacondarwin left a comment

Choose a reason for hiding this comment

petebacondarwin Nov 2, 2020

Choose a reason for hiding this comment

petebacondarwin Nov 2, 2020

Choose a reason for hiding this comment

JoostK Nov 2, 2020

Choose a reason for hiding this comment

petebacondarwin Nov 2, 2020

Choose a reason for hiding this comment

petebacondarwin commented Nov 2, 2020

petebacondarwin left a comment

Choose a reason for hiding this comment

petebacondarwin Nov 2, 2020

Choose a reason for hiding this comment

JoostK Nov 2, 2020

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

petebacondarwin left a comment

Choose a reason for hiding this comment

AndrewKushnir commented Nov 3, 2020 • edited

AndrewKushnir commented Nov 4, 2020

AndrewKushnir commented Nov 4, 2020

angular-automatic-lock-bot bot commented Dec 5, 2020

AndrewKushnir commented Nov 3, 2020 •

edited