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

refactor(core): allow developers to select static-query migration strategy #29876

Closed

Conversation

@devversion
Copy link
Member

commented Apr 12, 2019

  • See individual commits

@devversion devversion requested review from angular/fw-compiler as code owners Apr 12, 2019

@ngbot ngbot bot added this to the needsTriage milestone Apr 12, 2019

@googlebot googlebot added the cla: yes label Apr 12, 2019

@devversion devversion force-pushed the devversion:static-query-strategy-prompt branch 3 times, most recently from d34c968 to 0b392d4 Apr 12, 2019

@devversion devversion requested review from angular/fw-integration as code owners Apr 13, 2019

@devversion devversion force-pushed the devversion:static-query-strategy-prompt branch 3 times, most recently from 3986080 to d851fb6 Apr 13, 2019

@devversion devversion removed the request for review from angular/fw-compiler Apr 15, 2019

@kara kara removed their assignment Apr 17, 2019

@kara

kara approved these changes Apr 17, 2019

Copy link
Contributor

left a comment

LGTM once message is fixed

@alexeagle
Copy link
Contributor

left a comment

looks like a clever way to make the prompts work from core

devversion added some commits Apr 12, 2019

refactor(core): allow developers to select static-query migration str…
…ategy

Currently there are two available migration strategies for the `static-query`
schematic. Both have benefits and negatives which depend on what the
developer prefers. Since we can't decide which migration strategy is the
best for a given project, the developer should be able to select a specific
strategy through a simple choice prompt.

In order to be able to use prompts in a migration schematic, we need to
take advantage of the "inquirer" package which is also used by the CLI
schematic prompts (schematic prompts are usually only statically defined
in the schema). Additionally the schematic needs to be made "async"
because with prompts the schematic can no longer execute synchronously
without implementing some logic that blocks the execution.
refactor(core): static-query template strategy should not parse style…
…sheets

Currently the `template-strategy` for the static query migration uses the
Angular compiler in order to determine the query timing. This is problematic
as the AngularCompilerProgram also collects metadata for referenced
component stylesheets which aren't necessarily present. e.g. in a CLI
project the component can reference a Sass file. It's not guaranteed
that the standalone Angular compiler plugin supports Sass without
custom logic that is brought in by the Angular CLI webpack plugin.

In order to avoid any failures for invalid stylesheets, we just disable
normalizing of all referenced stylesheets.
fix(bazel): do not typecheck core schematic files
Currently for Angular Bazel projects, NGC needs to be run in the
"postinstall" NPM script in order to generate required summary files.

We need to update the postinstall `tsconfig` to not check/re-build the
`@angular/core` schematic code which has transitive dependencies
which are only available inside of a CLI project. As this is not guaranteed
to be the case with Angular Bazel projects, we need to make sure that
we don't check/re-build these files.

@devversion devversion force-pushed the devversion:static-query-strategy-prompt branch from d851fb6 to bf7dd9b Apr 17, 2019

@benlesh

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

@benlesh

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

Issues syncing this in google3, will discuss with other googlers.

@benlesh

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

Was able to fix google3 downstream. Good to merge.

@benlesh benlesh closed this in 2ba799d Apr 19, 2019

benlesh added a commit that referenced this pull request Apr 19, 2019

refactor(core): allow developers to select static-query migration str…
…ategy (#29876)

Currently there are two available migration strategies for the `static-query`
schematic. Both have benefits and negatives which depend on what the
developer prefers. Since we can't decide which migration strategy is the
best for a given project, the developer should be able to select a specific
strategy through a simple choice prompt.

In order to be able to use prompts in a migration schematic, we need to
take advantage of the "inquirer" package which is also used by the CLI
schematic prompts (schematic prompts are usually only statically defined
in the schema). Additionally the schematic needs to be made "async"
because with prompts the schematic can no longer execute synchronously
without implementing some logic that blocks the execution.

PR Close #29876

benlesh added a commit that referenced this pull request Apr 19, 2019

refactor(core): static-query template strategy should not parse style…
…sheets (#29876)

Currently the `template-strategy` for the static query migration uses the
Angular compiler in order to determine the query timing. This is problematic
as the AngularCompilerProgram also collects metadata for referenced
component stylesheets which aren't necessarily present. e.g. in a CLI
project the component can reference a Sass file. It's not guaranteed
that the standalone Angular compiler plugin supports Sass without
custom logic that is brought in by the Angular CLI webpack plugin.

In order to avoid any failures for invalid stylesheets, we just disable
normalizing of all referenced stylesheets.

PR Close #29876

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

fix(bazel): do not typecheck core schematic files (angular#29876)
Currently for Angular Bazel projects, NGC needs to be run in the
"postinstall" NPM script in order to generate required summary files.

We need to update the postinstall `tsconfig` to not check/re-build the
`@angular/core` schematic code which has transitive dependencies
which are only available inside of a CLI project. As this is not guaranteed
to be the case with Angular Bazel projects, we need to make sure that
we don't check/re-build these files.

PR Close angular#29876

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

refactor(core): allow developers to select static-query migration str…
…ategy (angular#29876)

Currently there are two available migration strategies for the `static-query`
schematic. Both have benefits and negatives which depend on what the
developer prefers. Since we can't decide which migration strategy is the
best for a given project, the developer should be able to select a specific
strategy through a simple choice prompt.

In order to be able to use prompts in a migration schematic, we need to
take advantage of the "inquirer" package which is also used by the CLI
schematic prompts (schematic prompts are usually only statically defined
in the schema). Additionally the schematic needs to be made "async"
because with prompts the schematic can no longer execute synchronously
without implementing some logic that blocks the execution.

PR Close angular#29876

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

refactor(core): static-query template strategy should not parse style…
…sheets (angular#29876)

Currently the `template-strategy` for the static query migration uses the
Angular compiler in order to determine the query timing. This is problematic
as the AngularCompilerProgram also collects metadata for referenced
component stylesheets which aren't necessarily present. e.g. in a CLI
project the component can reference a Sass file. It's not guaranteed
that the standalone Angular compiler plugin supports Sass without
custom logic that is brought in by the Angular CLI webpack plugin.

In order to avoid any failures for invalid stylesheets, we just disable
normalizing of all referenced stylesheets.

PR Close angular#29876
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.