Skip to content

refactor(ivy): remove LNode.tNode#25958

Closed
kara wants to merge 1 commit into
angular:masterfrom
kara:node.tNode
Closed

refactor(ivy): remove LNode.tNode#25958
kara wants to merge 1 commit into
angular:masterfrom
kara:node.tNode

Conversation

@kara
Copy link
Copy Markdown
Contributor

@kara kara commented Sep 14, 2018

This PR removes LNode.tNode.

Notes:

  • It migrates many of our main utility functions to using TNode args instead of LNode args where possible, adding as an additional argument where LNodes would be difficult to look up.
  • There is now a getTNode(index) helper that allows lookup via index in TView.data.
  • LViewData[HOST_NODE] now stores TNode instances instead of LNode instances. There is a new getHostElementNode() helper in node_manipulation.ts to get component host LNodes where necessary.
  • There's also some minor refactoring to share more code where I saw some duplication while re-wiring args. e.g. createRootComponent, getHighestElementContainer, getContainerRenderParent

Copy link
Copy Markdown
Contributor Author

@kara kara Sep 14, 2018

Choose a reason for hiding this comment

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

This is brought in by createLNode and also brings in 3 or 4 other small functions. I think we will be able to remove them as soon as we do the injector refactor.

Some of the other added fns in here are okay because I've broken existing code up into smaller functions to share more code.

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.

SGTM

@kara kara added state: WIP refactoring Issue that involves refactoring or code-cleanup target: major This PR is targeted for the next major release and removed cla: yes labels Sep 14, 2018
@kara kara requested a review from mhevery September 14, 2018 07:18
@mary-poppins
Copy link
Copy Markdown

You can preview e9be4c9 at https://pr25958-e9be4c9.ngbuilds.io/.

@kara kara added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 14, 2018
@mary-poppins
Copy link
Copy Markdown

You can preview 08f835c at https://pr25958-08f835c.ngbuilds.io/.

@kara
Copy link
Copy Markdown
Contributor Author

kara commented Sep 14, 2018

presubmit

Comment thread packages/core/src/render3/component_ref.ts Outdated
Comment thread packages/core/src/render3/di.ts
@kara kara added the action: merge The PR is ready for merge by the caretaker label Sep 14, 2018
@benlesh benlesh closed this in aedebaf Sep 14, 2018
@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 14, 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 refactoring Issue that involves refactoring or code-cleanup target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants