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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy stat 8000 ement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

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

… 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
…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
…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
`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
…38707)

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

PR Close angular#38707
)

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

PR Close angular#38707
…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
…#38707)

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

PR Close angular#38707
…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.
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
Development

Successfully merging this pull request may close these issues.

3 participants
0