Skip to content

fix(core): require 'static' flag on queries in typings - #30639

Closed
alxhub wants to merge 1 commit into
angular:masterfrom
alxhub:v8/static-queries
Closed

alxhub wants to merge 1 commit into
angular:masterfrom
alxhub:v8/static-queries

Conversation

@alxhub

@alxhub alxhub commented May 23, 2019 •

Copy link
Copy Markdown
Member

This commit makes the static flag on @ViewChild and @ContentChild required.

BREAKING CHANGE

Starting with v8.0, Angular requires that all @ViewChild and @ContentChild
queries have a 'static' flag specifying whether the query is 'static' or
'dynamic'. The compiler previously sorted queries automatically, but in
8.0 developers are required to explicitly specify which behavior is wanted.

@ViewChildren and @ContentChildren queries are always dynamic, and so are
unaffected.

See https://angular.io/guide/static-query-migration for more details (link not functional yet)

@alxhub
alxhub requested review from a team May 23, 2019 18:43
@alxhub
alxhub force-pushed the v8/static-queries branch from 1a197fb to 7c62398 Compare May 23, 2019 18:51
@alxhub
alxhub requested a review from a team May 23, 2019 18:51
@alxhub alxhub added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels May 23, 2019
@alxhub
alxhub force-pushed the v8/static-queries branch 2 times, most recently from b21f401 to e69763e Compare May 23, 2019 19:51
@alxhub
alxhub requested a review from a team May 23, 2019 19:51
@alxhub alxhub mentioned this pull request May 23, 2019
@alxhub

alxhub commented May 23, 2019

Copy link
Copy Markdown
Member Author

Presubmit

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kara kara added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label May 23, 2019
@kara

kara commented May 23, 2019

Copy link
Copy Markdown
Contributor

merge assistance: global approval

@kara kara added action: presubmit The PR is in need of a google3 presubmit and removed merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels May 23, 2019
@kara

kara commented May 23, 2019

Copy link
Copy Markdown
Contributor

Note: needs global TAP presubmit first

@alxhub alxhub added state: blocked and removed action: merge The PR is ready for merge by the caretaker labels May 23, 2019
This commit makes the static flag on @ViewChild and @ContentChild required.

BREAKING CHANGE:

In Angular version 8, it's required that all @ViewChild and @ContentChild
queries have a 'static' flag specifying whether the query is 'static' or
'dynamic'. The compiler previously sorted queries automatically, but in
8.0 developers are required to explicitly specify which behavior is wanted.
This is a temporary requirement as part of a migration; see
https://angular.io/guide/static-query-migration for more details.

@ViewChildren and @ContentChildren queries are always dynamic, and so are
unaffected.
@alxhub
alxhub force-pushed the v8/static-queries branch from e69763e to c06c972 Compare May 23, 2019 23:38
@alxhub
alxhub requested review from a team May 23, 2019 23:38

@petebacondarwin petebacondarwin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there also be an update to the cheatsheet? https://github.com/angular/angular/blob/master/aio/content/guide/cheatsheet.md

@petebacondarwin petebacondarwin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved from docs-infra POV

@alxhub alxhub added action: merge The PR is ready for merge by the caretaker 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: presubmit The PR is in need of a google3 presubmit state: blocked labels May 24, 2019
@ngbot

ngbot Bot commented May 24, 2019

Copy link
Copy Markdown

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci-codefresh-bazel" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@alxhub

alxhub commented May 24, 2019

Copy link
Copy Markdown
Member Author

Caretaker: the codefresh status appears to be out of date - the error shown was for a previous version of this commit. I don't know how to restart it.

@matsko matsko closed this in 84dd267 May 24, 2019
alxhub added a commit to alxhub/angular that referenced this pull request May 24, 2019
This fixes a collision between angular#30639 and angular#30543 where the latter added
usages of @ViewChild without the static flag present, but the former
made the flag required.
@angular-automatic-lock-bot

Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants