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

Preserve metadata during AOT compilation via setClassMetadata #26860

Closed
wants to merge 6 commits into
base: master
from

Conversation

@alxhub
Contributor

alxhub commented Oct 30, 2018

No description provided.

alxhub added some commits Oct 30, 2018

feat(compiler): ability to mark an InvokeFunctionExpr as pure
Uglify and other tree-shakers attempt to determine if the invocation
of a function is side-effectful, and remove it if so (and the result
is unused). A /*@__PURE__*/ annotation on the call site can be used
to hint to the optimizer that the invocation has no side effects and
is safe to tree-shake away.

This commit adds a 'pure' flag to the output AST function call node,
which can be used to signal to downstream emitters that a pure
annotation should be added. It also modifies ngtsc's emitter to
emit an Uglify pure annotation when this flag is set.

Testing strategy: this will be tested via its consumers, by asserting
that pure functions are translated with the correct comment.
refactor(ivy): use wrapped metadata in all DecoratorHandlers
Previously, the Directive, Injectable, and Pipe DecoratorHandlers were
directly returning @angular/compiler metadata from their analyze() steps.
This precludes returning any additional information along with that
metadata. This commit introduces a wrapper interface for these handlers,
opening the door for additional information to be returned from analyze().

Testing strategy: this is a refactor commit, existing test coverage is
sufficient.
feat(ivy): capture the identifier of a decorator during reflection
Previously the ReflectionHost API only returned the names of decorators
and not a reference to their TypeScript Identifier. This commit adds
the identifier itself, so that a consumer can write references to the
decorator.

Testing strategy: this commit is trivial, and the functionality will be
exercised by downstream tests.

@googlebot googlebot added the cla: yes label Oct 30, 2018

@alxhub alxhub requested review from benlesh and mhevery Oct 30, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 30, 2018

@alexeagle

approval for the BUILD.bazel change

@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 31, 2018

alxhub added some commits Oct 30, 2018

feat(ivy): setClassMetadata() for assigning decorator metadata
This commit introduces the setClassMetadata() private function, which
adds metadata to a type in a way that can be accessed via Angular's
ReflectionCapabilities. Currently, it writes to static fields as if
the metadata being added was downleveled from decorators by tsickle.

The plan is for ngtsc to emit code which calls this function, passing
metadata on to the runtime for testing purposes. Calls to this function
would then be tree-shaken away for production bundles.

Testing strategy: proper operation of this function will be an integral
part of TestBed metadata overriding. Angular core tests will fail if this
is broken.
feat(ivy): generator of setClassMetadata statements for Angular types
This commit introduces generateSetClassMetadataCall(), an API in ngtsc
for generating calls to setClassMetadata() for a given declaration. The
reflection API is used to enumerate Angular decorators on the declaration,
which are converted to a format that ReflectionCapabilities can understand.
The reflection metadata is then patched onto the declared type via a call
to setClassMetadata().

This is simply a utility, a future commit invokes this utility for
each DecoratorHandler.

Testing strategy: tests are included which exercise generateSetClassMetadata
in isolation.
@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 31, 2018

feat(ivy): emit metadata along with all Angular types
This commit causes a call to setClassMetadata() to be emitted for every
type being compiled by ngtsc (every Angular type). With this metadata,
the TestBed should be able to recompile these classes when overriding
decorator information.

Testing strategy: Tests in the previous commit for
generateSetClassMetadataCall() verify that the metadata as generated is
correct. This commit enables the generation for each DecoratorHandler,
and a test is added to ngtsc_spec to verify all decorated types have
metadata generated for them.
@mary-poppins

This comment has been minimized.

mary-poppins commented Oct 31, 2018

@benlesh

This comment has been minimized.

Contributor

benlesh commented Oct 31, 2018

When looking at core:test, the PR improves the situation quite a bit:

master
1264 specs, 776 failures, 2 pending specs

this PR
1264 specs, 539 failures, 2 pending specs

237 fixed tests

@alxhub

This comment has been minimized.

Contributor

alxhub commented Oct 31, 2018

Caretaker: Pull Approve is not cooperating. Please hit it over the head.

@jasonaden

Try to get pull approve to pass this on.

@matsko matsko closed this in 4dfa71f Oct 31, 2018

matsko added a commit that referenced this pull request Oct 31, 2018

feat(ivy): capture the identifier of a decorator during reflection (#…
…26860)

Previously the ReflectionHost API only returned the names of decorators
and not a reference to their TypeScript Identifier. This commit adds
the identifier itself, so that a consumer can write references to the
decorator.

Testing strategy: this commit is trivial, and the functionality will be
exercised by downstream tests.

PR Close #26860

matsko added a commit that referenced this pull request Oct 31, 2018

refactor(ivy): use wrapped metadata in all DecoratorHandlers (#26860)
Previously, the Directive, Injectable, and Pipe DecoratorHandlers were
directly returning @angular/compiler metadata from their analyze() steps.
This precludes returning any additional information along with that
metadata. This commit introduces a wrapper interface for these handlers,
opening the door for additional information to be returned from analyze().

Testing strategy: this is a refactor commit, existing test coverage is
sufficient.

PR Close #26860

matsko added a commit that referenced this pull request Oct 31, 2018

feat(ivy): setClassMetadata() for assigning decorator metadata (#26860)
This commit introduces the setClassMetadata() private function, which
adds metadata to a type in a way that can be accessed via Angular's
ReflectionCapabilities. Currently, it writes to static fields as if
the metadata being added was downleveled from decorators by tsickle.

The plan is for ngtsc to emit code which calls this function, passing
metadata on to the runtime for testing purposes. Calls to this function
would then be tree-shaken away for production bundles.

Testing strategy: proper operation of this function will be an integral
part of TestBed metadata overriding. Angular core tests will fail if this
is broken.

PR Close #26860

matsko added a commit that referenced this pull request Oct 31, 2018

feat(ivy): generator of setClassMetadata statements for Angular types (
…#26860)

This commit introduces generateSetClassMetadataCall(), an API in ngtsc
for generating calls to setClassMetadata() for a given declaration. The
reflection API is used to enumerate Angular decorators on the declaration,
which are converted to a format that ReflectionCapabilities can understand.
The reflection metadata is then patched onto the declared type via a call
to setClassMetadata().

This is simply a utility, a future commit invokes this utility for
each DecoratorHandler.

Testing strategy: tests are included which exercise generateSetClassMetadata
in isolation.

PR Close #26860

matsko added a commit that referenced this pull request Oct 31, 2018

feat(ivy): emit metadata along with all Angular types (#26860)
This commit causes a call to setClassMetadata() to be emitted for every
type being compiled by ngtsc (every Angular type). With this metadata,
the TestBed should be able to recompile these classes when overriding
decorator information.

Testing strategy: Tests in the previous commit for
generateSetClassMetadataCall() verify that the metadata as generated is
correct. This commit enables the generation for each DecoratorHandler,
and a test is added to ngtsc_spec to verify all decorated types have
metadata generated for them.

PR Close #26860

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(compiler): ability to mark an InvokeFunctionExpr as pure (angula…
…r#26860)

Uglify and other tree-shakers attempt to determine if the invocation
of a function is side-effectful, and remove it if so (and the result
is unused). A /*@__PURE__*/ annotation on the call site can be used
to hint to the optimizer that the invocation has no side effects and
is safe to tree-shake away.

This commit adds a 'pure' flag to the output AST function call node,
which can be used to signal to downstream emitters that a pure
annotation should be added. It also modifies ngtsc's emitter to
emit an Uglify pure annotation when this flag is set.

Testing strategy: this will be tested via its consumers, by asserting
that pure functions are translated with the correct comment.

PR Close angular#26860

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(ivy): capture the identifier of a decorator during reflection (a…
…ngular#26860)

Previously the ReflectionHost API only returned the names of decorators
and not a reference to their TypeScript Identifier. This commit adds
the identifier itself, so that a consumer can write references to the
decorator.

Testing strategy: this commit is trivial, and the functionality will be
exercised by downstream tests.

PR Close angular#26860

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

refactor(ivy): use wrapped metadata in all DecoratorHandlers (angular…
…#26860)

Previously, the Directive, Injectable, and Pipe DecoratorHandlers were
directly returning @angular/compiler metadata from their analyze() steps.
This precludes returning any additional information along with that
metadata. This commit introduces a wrapper interface for these handlers,
opening the door for additional information to be returned from analyze().

Testing strategy: this is a refactor commit, existing test coverage is
sufficient.

PR Close angular#26860

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(ivy): setClassMetadata() for assigning decorator metadata (angul…
…ar#26860)

This commit introduces the setClassMetadata() private function, which
adds metadata to a type in a way that can be accessed via Angular's
ReflectionCapabilities. Currently, it writes to static fields as if
the metadata being added was downleveled from decorators by tsickle.

The plan is for ngtsc to emit code which calls this function, passing
metadata on to the runtime for testing purposes. Calls to this function
would then be tree-shaken away for production bundles.

Testing strategy: proper operation of this function will be an integral
part of TestBed metadata overriding. Angular core tests will fail if this
is broken.

PR Close angular#26860

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(ivy): generator of setClassMetadata statements for Angular types (
…angular#26860)

This commit introduces generateSetClassMetadataCall(), an API in ngtsc
for generating calls to setClassMetadata() for a given declaration. The
reflection API is used to enumerate Angular decorators on the declaration,
which are converted to a format that ReflectionCapabilities can understand.
The reflection metadata is then patched onto the declared type via a call
to setClassMetadata().

This is simply a utility, a future commit invokes this utility for
each DecoratorHandler.

Testing strategy: tests are included which exercise generateSetClassMetadata
in isolation.

PR Close angular#26860

sculove added a commit to sculove/angular that referenced this pull request Nov 2, 2018

feat(ivy): emit metadata along with all Angular types (angular#26860)
This commit causes a call to setClassMetadata() to be emitted for every
type being compiled by ngtsc (every Angular type). With this metadata,
the TestBed should be able to recompile these classes when overriding
decorator information.

Testing strategy: Tests in the previous commit for
generateSetClassMetadataCall() verify that the metadata as generated is
correct. This commit enables the generation for each DecoratorHandler,
and a test is added to ngtsc_spec to verify all decorated types have
metadata generated for them.

PR Close angular#26860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment