Skip to content
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(core): static-query migration should gracefully exit if AOT compiler throws #30269

Closed

Conversation

@devversion
Copy link
Member

commented May 5, 2019

  • See individual commits (except first one)

Note: Blocked on #30254

@googlebot googlebot added the cla: yes label May 5, 2019

@devversion devversion force-pushed the devversion:refactor/core-static-query-safety branch from 6564025 to af2def7 May 5, 2019

@ngbot ngbot bot added this to the needsTriage milestone May 5, 2019

@devversion devversion marked this pull request as ready for review May 5, 2019

@devversion devversion requested a review from angular/fw-core as a code owner May 5, 2019

@CaerusKaru
Copy link
Member

left a comment

LGTM

@kara

kara approved these changes May 8, 2019

Copy link
Contributor

left a comment

LGTM, found a typo though

@kara kara removed their assignment May 8, 2019

@kara

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

@devversion Also could you rename the commit to fix(core)? Refactors technically don't change behavior.

@devversion devversion force-pushed the devversion:refactor/core-static-query-safety branch from af2def7 to d3d1062 May 8, 2019

@devversion

This comment has been minimized.

Copy link
Member Author

commented May 8, 2019

@kara Done. I wasn't too sure if we want to list these changes in the changelog.

@devversion devversion changed the title refactor(core): static-query migration should gracefully exit if AOT compiler throws fix(core): static-query migration should gracefully exit if AOT compiler throws May 8, 2019

devversion added some commits May 5, 2019

fix(core): static-query migration fails with default parameter values
Currently when someone has a call expression within the `ngOnInit` call
and we try to peek into that function with respect to the current function
context, the schematic errors because a call expression argument is
undefined. This is valid because the target function declaration defines
that parameter with a default value. In order to fix this, we need to
respect parameter default values.
fix(core): migrations not always migrating all files
In an Angular CLI project scenario where projects only reference
top-level source-files through the `tsconfig` `files` option, we currently
do not migrate referenced source-files. This can be fixed checking all
referenced source-files which aren't coming from an external library.

This is similar to how `tslint` determines project source-files.
fix(core): static-query migration should gracefully exit if AOT compi…
…ler throws

The static-query template strategy leverages the AOT compiler
in order to determine the query timing. Unfortunately the AOT
compiler has open bugs that can cause unexpected failures which
make the template strategy unusable in rare cases. These rare
exceptions need to be handled gracefully in order to avoid confusion
and to provide a more smooth migration.

Additionally migration strategy setup failures are now reported with
stack traces as the `ng update` command does not print stack traces.
This makes it easier to reproduce and report migration issues.

@devversion devversion force-pushed the devversion:refactor/core-static-query-safety branch from d3d1062 to 2c2bafb May 8, 2019

@alxhub

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

Presubmit (note that this will have a failure because the presubmit CL does not have the manual g3 change needed for the previous PR)

alxhub added a commit that referenced this pull request May 8, 2019

fix(core): static-query migration fails with default parameter values (
…#30269)

Currently when someone has a call expression within the `ngOnInit` call
and we try to peek into that function with respect to the current function
context, the schematic errors because a call expression argument is
undefined. This is valid because the target function declaration defines
that parameter with a default value. In order to fix this, we need to
respect parameter default values.

PR Close #30269

alxhub added a commit that referenced this pull request May 8, 2019

fix(core): migrations not always migrating all files (#30269)
In an Angular CLI project scenario where projects only reference
top-level source-files through the `tsconfig` `files` option, we currently
do not migrate referenced source-files. This can be fixed checking all
referenced source-files which aren't coming from an external library.

This is similar to how `tslint` determines project source-files.

PR Close #30269

alxhub added a commit that referenced this pull request May 8, 2019

fix(core): static-query migration should gracefully exit if AOT compi…
…ler throws (#30269)

The static-query template strategy leverages the AOT compiler
in order to determine the query timing. Unfortunately the AOT
compiler has open bugs that can cause unexpected failures which
make the template strategy unusable in rare cases. These rare
exceptions need to be handled gracefully in order to avoid confusion
and to provide a more smooth migration.

Additionally migration strategy setup failures are now reported with
stack traces as the `ng update` command does not print stack traces.
This makes it easier to reproduce and report migration issues.

PR Close #30269

@alxhub alxhub closed this in 6357d4a May 8, 2019

alxhub added a commit that referenced this pull request May 8, 2019

fix(core): migrations not always migrating all files (#30269)
In an Angular CLI project scenario where projects only reference
top-level source-files through the `tsconfig` `files` option, we currently
do not migrate referenced source-files. This can be fixed checking all
referenced source-files which aren't coming from an external library.

This is similar to how `tslint` determines project source-files.

PR Close #30269

alxhub added a commit that referenced this pull request May 8, 2019

fix(core): static-query migration should gracefully exit if AOT compi…
…ler throws (#30269)

The static-query template strategy leverages the AOT compiler
in order to determine the query timing. Unfortunately the AOT
compiler has open bugs that can cause unexpected failures which
make the template strategy unusable in rare cases. These rare
exceptions need to be handled gracefully in order to avoid confusion
and to provide a more smooth migration.

Additionally migration strategy setup failures are now reported with
stack traces as the `ng update` command does not print stack traces.
This makes it easier to reproduce and report migration issues.

PR Close #30269

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

fix(core): static-query migration fails with default parameter values (
…angular#30269)

Currently when someone has a call expression within the `ngOnInit` call
and we try to peek into that function with respect to the current function
context, the schematic errors because a call expression argument is
undefined. This is valid because the target function declaration defines
that parameter with a default value. In order to fix this, we need to
respect parameter default values.

PR Close angular#30269

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

fix(core): migrations not always migrating all files (angular#30269)
In an Angular CLI project scenario where projects only reference
top-level source-files through the `tsconfig` `files` option, we currently
do not migrate referenced source-files. This can be fixed checking all
referenced source-files which aren't coming from an external library.

This is similar to how `tslint` determines project source-files.

PR Close angular#30269

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

fix(core): static-query migration should gracefully exit if AOT compi…
…ler throws (angular#30269)

The static-query template strategy leverages the AOT compiler
in order to determine the query timing. Unfortunately the AOT
compiler has open bugs that can cause unexpected failures which
make the template strategy unusable in rare cases. These rare
exceptions need to be handled gracefully in order to avoid confusion
and to provide a more smooth migration.

Additionally migration strategy setup failures are now reported with
stack traces as the `ng update` command does not print stack traces.
This makes it easier to reproduce and report migration issues.

PR Close angular#30269
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.