8000 Previous or parent t nodes always matches tview 10.1.x by mhevery · Pull Request #39029 · angular/angular · GitHub
[go: up one dir, main page]
Skip to content

Previous or parent t nodes always matches tview 10.1.x #39029

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Sep 29, 2020
Merged

Previous or parent t nodes always matches tview 10.1.x #39029

merged 11 commits into from
Sep 29, 2020

Conversation

mhevery
Copy link
Contributor
@mhevery mhevery commented Sep 28, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from IgorMinar September 28, 2020 20:35
@mhevery
docs(core): correct instructions for running component repo locally (a…
689651b 
…ngular#38707)

PR Close angular#38707
@mhevery
refactor(core): Ensure that previousOrParentTNode always belongs to…
33aaa9e 
… current `TView`. (angular#38707)

`previousOrParentTNode` stores current `TNode`. Due to inconsistent
implementation the value stored would sometimes belong to the current
`TView` and sometimes to the parent. We have extra logic which accounts
for it. A better solution is to just ensure that `previousOrParentTNode`
always belongs to current `TNode`. This simplifies the mental model
and cleans up some code.

PR Close angular#38707
@mhevery
refactor(core): Remove host TNode from getOrCreateTNode (angular#…
bc6ff77 
…38707)

Host `TNode` was passed into `getOrCreateTNode` just so that we can
compute weather or not we are a root node. This was needed because
`previousOrParentTNode` could have `TNode` from `TView` other then
current `TView`. This is confusing mental model. Previous change
ensured that `previousOrParentTNode` must always be part of `TView`,
which enabled this change to remove the unneeded argument.

PR Close angular#38707
@mhevery
refactor(core): Add injector debug information to LViewDebug (angul…
b613639 
…ar#38707)

Extended the `LViewDebug` to display node-injector information for each
node.

PR Close angular#38707
@mhevery mhevery force-pushed the previousOrParentTNodes-always-matches-tview-10.1.x branch 2 times, most recently from e287005 to 108ef65 Compare September 28, 2020 22:47
@mhevery
refactor(core): Remove reliance on TNodeType.View. (angular#38707)
4645f43 
`TNodeType.View` was created to support inline views. That feature did
not materialize and we have since removed the instructions for it, leave
 an unneeded `TNodeType.View` which was still used in a very
 inconsistent way. This change no longer created `TNodeType.View` (and
 there will be a follow up chang to completely remove it.)

Also simplified the mental model so that `LView[HOST]`/`LView[T_HOST]`
always point to the insertion location of the `LView`.

PR Close angular#38707
@mhevery
refactor(core): Remove TViewNode as it is no longer used. (angular#…
ed35adb 
…38707)

Previous commit change the logic to not rely on the `TViewNode` this
change removes it entirely.

PR Close angular#38707
@mhevery
refactor(core): Rename TView.node to TView.declTNode. (angular#38707
15fa4bb 
)

The value stored in `TView.node` is really the declaration `TNode`,
therefore renaming to make it more explicit.

PR Close angular#38707
@mhevery
refactor(core): change getPreviousOrParentTNode to return `TNode|nu…
0fa208f 
…ll` (angular#38707)

This change makes `getPreviousOrParentTNode` return `TNode|null` (rather
than just `TNode`) which is more reflective of the reality. The
`getPreviousOrParentTNode` can be `null` upon entering the `LView`.

PR Close angular#38707
@mhevery
refactor(core): renamed previousOrParent to currentTNode (angular…
65d4e7a 
…#38707)

The previous name of `previousOrParent` was confusing. Changed the
terminology to `currentTNode`.

PR Close angular#38707
@mhevery
docs(core): Update instructions on updating symbol tests (angular#38707)
9891cef 
PR Close angular#38707
@mhevery
refactor(core): Create NodeInjectorOffset type which better describ…
69302ad 
…es NodeInjector (angular#38707)

`NodeInjector` is store in expando as a list of values in an array. The
offset constant into the array have been brought together into a single
`NodeInjectorOffset` enum with better documentation explaining their usage.

PR Close angular#38707
@mhevery mhevery force-pushed the previousOrParentTNodes-always-matches-tview-10.1.x branch from 108ef65 to 69302ad Compare September 28, 2020 23:21
@mhevery mhevery added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Sep 28, 2020
@mhevery
Copy link
Contributor Author
mhevery commented Sep 28, 2020

CARETAKER: This is a 10.1.x branch of #38707. Don't merge it using caretaker merge script because this PR already has PR closing comments in as it was cherry-picked from master. Merge it manually.

@alxhub alxhub merged commit 69302ad into angular:10.1.x Sep 29, 2020
@angular-automatic-lock-bot
Copy link

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 Oct 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@IgorMinar IgorMinar Awaiting requested review from IgorMinar

Assignees
No one assigned
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: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mhevery @alxhub @googlebot
0