Implement HostSingleton type #25010
Implement HostSingleton type #25010
Conversation
| while (attributes.length) { | ||
| domElement.removeAttribute(attributes[0].name); | ||
| } | ||
| setInitialProperties(domElement, tag, rawProps, rootContainerElement); |
There was a problem hiding this comment.
This function is no longer being inlined on the main path. Seems unfortunate to duplicate it but maybe necessary
|
|
||
| expect(body === testDocument.body).toBe(true); | ||
| }); | ||
|
|
||
| it('should not be able to unmount component from document node', () => { | ||
| it('should be able to unmount component from document node, but leaves persistent nodes intact', () => { |
There was a problem hiding this comment.
This test was incorrectly described for 5 years. it says you shouldn't be able to do something but it asserts that you can (b/c fiber). I've updated the test description and modified it based on the new constraints of insertion markers for head/body
| export function resetProperties( | ||
| domElement: Element, | ||
| tag: string, | ||
| rawProps: Object, | ||
| rootContainerElement: Element | Document | DocumentFragment, | ||
| ): void { | ||
| const attributes = domElement.attributes; | ||
| while (attributes.length) { | ||
| domElement.removeAttribute(attributes[0].name); | ||
| } | ||
| setInitialProperties(domElement, tag, rawProps, rootContainerElement); | ||
| } | ||
|
|
There was a problem hiding this comment.
This concept maybe needs more refinement. What we want is to put the already existing node into a state in which it would have been constructed fresh from React. At the moment this does not deal with listeners including ones set by React on previous Placements.
Practically it isn't common to do many placements for html, head, or body. The most common reasons we'd get here are
- failed hydration falling back to client render
- client only apps that bind to the document (rare still probably)
| export function getInstanceFromNode(node: HTMLElement): null | Object { | ||
| return getClosestInstanceFromNode(node) || null; | ||
| } | ||
|
|
||
| export function preparePortalMount(portalInstance: Instance): void { | ||
| listenToAllSupportedEvents(portalInstance); | ||
| } | ||
|
|
||
| export function prepareScopeUpdate( | ||
| scopeInstance: ReactScopeInstance, | ||
| internalInstanceHandle: Object, | ||
| ): void { | ||
| if (enableScopeAPI) { | ||
| precacheFiberNode(internalInstanceHandle, scopeInstance); | ||
| } | ||
| } | ||
|
|
||
| export function getInstanceFromScope( | ||
| scopeInstance: ReactScopeInstance, | ||
| ): null | Object { | ||
| if (enableScopeAPI) { | ||
| return getFiberFromScopeInstance(scopeInstance); | ||
| } | ||
| return null; | ||
| } | ||
|
|
There was a problem hiding this comment.
These were simply relocated since they were in a section between hydration and test selectors.
| export function isPersistentInstance(instance: Instance | Container): boolean { | ||
| if (instance.nodeType === ELEMENT_NODE) { | ||
| return isPersistentInstanceType(instance.tagName.toLowerCase()); | ||
| } | ||
| return false; |
There was a problem hiding this comment.
I would like to unify this with isPersistentInstanceType but sometimes I have a type string and other times I have a Container and we can't get the type from a Container in the reconciler so I think we need both :(
| precacheFiberNode(internalInstanceHandle, element); | ||
| updateFiberProps(element, props); | ||
| } else { | ||
| element = fallbackInstance; |
There was a problem hiding this comment.
At the moment the fallback instance already has props set on it but that is technically wasted work almost all the time. we could defer it to here but the HostContext isn't built up in the commit phase so it's potentially harder to do here
|
In terms of naming, I don't think we can use |
| function commitReconciliationEffects(finishedWork: Fiber) { | ||
| function commitReconciliationEffects( | ||
| finishedWork: Fiber, | ||
| rootContainerInstance: Container, |
There was a problem hiding this comment.
Turns out, you don't need this. #25024
| @@ -223,7 +227,11 @@ if (supportsMutation) { | |||
| let node = workInProgress.child; | |||
| while (node !== null) { | |||
| if (node.tag === HostComponent || node.tag === HostText) { | |||
| appendInitialChild(parent, node.stateNode); | |||
| if (supportsHydration && node.flags & Placement) { | |||
There was a problem hiding this comment.
All of these are forked on supportsHydration but these don't have anything to do with hydration do they? E.g. I can render into document.body with createRoot and it's still relevant right?
There was a problem hiding this comment.
A cheat technique for now to opt out in non dom renderers. I can move to it’s own thing
There was a problem hiding this comment.
Yeah this does not depend on concurrent roots
There was a problem hiding this comment.
I think this will all need to go behind a feature flag because it's technically a breaking change. Doesn't this basically implement the whole new <head> semantics already except it also unmounts it?
| @@ -138,6 +141,8 @@ const SUSPENSE_END_DATA = '/$'; | |||
| const SUSPENSE_PENDING_START_DATA = '$?'; | |||
| const SUSPENSE_FALLBACK_START_DATA = '$!'; | |||
|
|
|||
| const INSERTION_MARKER = '%'; | |||
There was a problem hiding this comment.
It seems to me that all this insertion marker stuff is only needed because the React algorithm uses "insert before" instead of "insert after" - which is partially because that's what the only DOM APIs favored but also because we use linked lists that only go in one direction so we can't easily look backwards.
However, if it wasn't for that we'd have no need for these markers because we'd just insert after the last known node instead. It seems pretty inelegant to go through the hassle of mutating the DOM to insert visible placeholders when the algorithm should just be able to do that by itself.
There was a problem hiding this comment.
I updated the algo to not require any mutations. I think it is decent however lmk if the concerns of the speed of reading expando outweight the benefits of the new approach.
One thing I like about it is it also fixes portal insertion stability
gnoff
changed the title
gnoff
force-pushed
the
persistent-host-instances
branch
3 times, most recently
from
c68a06c
to
a00d190
Compare
gnoff
force-pushed
the
persistent-host-instances
branch
from
a00d190
to
272dce4
Compare
Background
For a variety of reasons
html,head, andbodynodes are special in the browser and historically there has not been much heed given to these elements in particular within react-dom which has led to some limitations of interoperability with externals systems such as 3rd party scripts and browser extensions. Additionally new features in React will require some special handling of these instances to be possible.The main issue has to do with instance lifecycles and React's total ownership of the nodes within it's tree. For the
headfor instance, if there are stylesheet links inserted by 3rd parties, React might unmount those nodes, or reinsert them somewhere else causing a new fetch and temporary style unloading. Additionally useInsertionEffect runs in the mutation phase but if we are going to be replacing the head it will be unmounted during the time these effects are run so you can't always inject styles when you expected to be able to.The historical advice has been to render into an element in the body that isn't like to be targeted by any external systems however this conflicts with the guidance for using streaming rendering available in React 18 where it is going to be common that React owns the entire document.
General Approach
To solve these issues, this PR introduces a new fiber type
HostSingleton. Currently only react-dom has an implementation for this type and all other renderers still only useHostComponent.HostSingletonsare placed individually, they will not be appended to a parent fiber even if they are part of a tree that is going to be Placed in the same commit.HostSingletonsdo not append anyHostComponentchildrenHostSingletonsif a Placement effect is needed a special Singleton placement method from the HostConfig is used. Child fibers of the fiber are then appended.HostSingletonis having Placement effects appended to it, it will use optional additional semantics for finding an insertion edge. This allows for non-react-controlled children to be siblings of Placed fibers without incorrect ordering.react-dom Approach
In the case of react-dom there are three singleton instances,
document.documentElement,document.head,document.body. If you render a<html /> | <head /> | <body />in your tree they each will bind to the already existing Element and not recreate a new one.Key Constraints:
insertion stability (Body and Head only)
In Body and Head, we expect there to be Nodes that were created by systems other than React. Most items can safely be removed once loaded because they are fire-and-forget, such as scripts. However
<style>and<link rel="stylesheet">nodes need to be retained for proper functioning of a site and as such we expect that these two instances will have a number of Nodes outside the purview of React's runtime which change insertion semantics.