-
Notifications
You must be signed in to change notification settings - Fork 51k
Store instance handles in an internal map behind flag #35053
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 statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+91
−11
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,8 @@ import {getParentHydrationBoundary} from './ReactFiberConfigDOM'; | |
|
|
||
| import {enableScopeAPI} from 'shared/ReactFeatureFlags'; | ||
|
|
||
| import {enableInternalInstanceMap} from 'shared/ReactFeatureFlags'; | ||
|
|
||
| const randomKey = Math.random().toString(36).slice(2); | ||
| const internalInstanceKey = '__reactFiber$' + randomKey; | ||
| const internalPropsKey = '__reactProps$' + randomKey; | ||
|
|
@@ -49,7 +51,32 @@ const internalRootNodeResourcesKey = '__reactResources$' + randomKey; | |
| const internalHoistableMarker = '__reactMarker$' + randomKey; | ||
| const internalScrollTimer = '__reactScroll$' + randomKey; | ||
|
|
||
| type InstanceUnion = | ||
| | Instance | ||
| | TextInstance | ||
| | SuspenseInstance | ||
| | ActivityInstance | ||
| | ReactScopeInstance | ||
| | Container; | ||
|
|
||
| const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map; | ||
| const internalInstanceMap: | ||
| | WeakMap<InstanceUnion, Fiber> | ||
| | Map<InstanceUnion, Fiber> = new PossiblyWeakMap(); | ||
| const internalPropsMap: | ||
| | WeakMap<InstanceUnion, Props> | ||
| | Map<InstanceUnion, Props> = new PossiblyWeakMap(); | ||
|
|
||
| export function detachDeletedInstance(node: Instance): void { | ||
| if (enableInternalInstanceMap) { | ||
| internalInstanceMap.delete(node); | ||
| internalPropsMap.delete(node); | ||
| delete (node: any)[internalEventHandlersKey]; | ||
| delete (node: any)[internalEventHandlerListenersKey]; | ||
| delete (node: any)[internalEventHandlesSetKey]; | ||
| delete (node: any)[internalRootNodeResourcesKey]; | ||
| return; | ||
| } | ||
| // TODO: This function is only called on host components. I don't think all of | ||
| // these fields are relevant. | ||
| delete (node: any)[internalInstanceKey]; | ||
|
|
@@ -68,6 +95,10 @@ export function precacheFiberNode( | |
| | ActivityInstance | ||
| | ReactScopeInstance, | ||
| ): void { | ||
| if (enableInternalInstanceMap) { | ||
| internalInstanceMap.set(node, hostInst); | ||
| return; | ||
| } | ||
| (node: any)[internalInstanceKey] = hostInst; | ||
| } | ||
|
|
||
|
|
@@ -95,7 +126,12 @@ export function isContainerMarkedAsRoot(node: Container): boolean { | |
| // HostRoot back. To get to the HostRoot, you need to pass a child of it. | ||
| // The same thing applies to Suspense and Activity boundaries. | ||
| export function getClosestInstanceFromNode(targetNode: Node): null | Fiber { | ||
| let targetInst = (targetNode: any)[internalInstanceKey]; | ||
| let targetInst: void | Fiber; | ||
| if (enableInternalInstanceMap) { | ||
| targetInst = internalInstanceMap.get(((targetNode: any): InstanceUnion)); | ||
| } else { | ||
| targetInst = (targetNode: any)[internalInstanceKey]; | ||
| } | ||
| if (targetInst) { | ||
| // Don't return HostRoot, SuspenseComponent or ActivityComponent here. | ||
| return targetInst; | ||
|
|
@@ -112,9 +148,15 @@ export function getClosestInstanceFromNode(targetNode: Node): null | Fiber { | |
| // itself because the fibers are conceptually between the container | ||
| // node and the first child. It isn't surrounding the container node. | ||
| // If it's not a container, we check if it's an instance. | ||
| targetInst = | ||
| (parentNode: any)[internalContainerInstanceKey] || | ||
| (parentNode: any)[internalInstanceKey]; | ||
| if (enableInternalInstanceMap) { | ||
| targetInst = | ||
| (parentNode: any)[internalContainerInstanceKey] || | ||
| internalInstanceMap.get(((parentNode: any): InstanceUnion)); | ||
| } else { | ||
| targetInst = | ||
| (parentNode: any)[internalContainerInstanceKey] || | ||
| (parentNode: any)[internalInstanceKey]; | ||
| } | ||
| if (targetInst) { | ||
| // Since this wasn't the direct target of the event, we might have | ||
| // stepped past dehydrated DOM nodes to get here. However they could | ||
|
|
@@ -147,8 +189,10 @@ export function getClosestInstanceFromNode(targetNode: Node): null | Fiber { | |
| // have had an internalInstanceKey on it. | ||
| // Let's get the fiber associated with the SuspenseComponent | ||
| // as the deepest instance. | ||
| // $FlowFixMe[prop-missing] | ||
| const targetFiber = hydrationInstance[internalInstanceKey]; | ||
| const targetFiber = enableInternalInstanceMap | ||
| ? internalInstanceMap.get(hydrationInstance) | ||
| : // $FlowFixMe[prop-missing] | ||
| hydrationInstance[internalInstanceKey]; | ||
| if (targetFiber) { | ||
| return targetFiber; | ||
| } | ||
|
|
@@ -175,9 +219,16 @@ export function getClosestInstanceFromNode(targetNode: Node): null | Fiber { | |
| * instance, or null if the node was not rendered by this React. | ||
| */ | ||
| export function getInstanceFromNode(node: Node): Fiber | null { | ||
| const inst = | ||
| (node: any)[internalInstanceKey] || | ||
| (node: any)[internalContainerInstanceKey]; | ||
| let inst: void | null | Fiber; | ||
| if (enableInternalInstanceMap) { | ||
| inst = | ||
| internalInstanceMap.get(((node: any): InstanceUnion)) || | ||
| (node: any)[internalContainerInstanceKey]; | ||
|
Comment on lines
+225
to
+226
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. compare to here for example |
||
| } else { | ||
| inst = | ||
| (node: any)[internalInstanceKey] || | ||
| (node: any)[internalContainerInstanceKey]; | ||
| } | ||
| if (inst) { | ||
| const tag = inst.tag; | ||
| if ( | ||
|
|
@@ -226,16 +277,24 @@ export function getFiberCurrentPropsFromNode( | |
| | TextInstance | ||
| | SuspenseInstance | ||
| | ActivityInstance, | ||
| ): Props { | ||
| ): Props | null { | ||
| if (enableInternalInstanceMap) { | ||
| return internalPropsMap.get(node) || null; | ||
| } | ||
| return (node: any)[internalPropsKey] || null; | ||
| } | ||
|
|
||
| export function updateFiberProps(node: Instance, props: Props): void { | ||
| if (enableInternalInstanceMap) { | ||
| internalPropsMap.set(node, props); | ||
| return; | ||
| } | ||
| (node: any)[internalPropsKey] = props; | ||
| } | ||
|
|
||
| export function getEventListenerSet(node: EventTarget): Set<string> { | ||
| let elementListenerSet = (node: any)[internalEventHandlersKey]; | ||
| let elementListenerSet: Set<string> | void; | ||
| elementListenerSet = (node: any)[internalEventHandlersKey]; | ||
| if (elementListenerSet === undefined) { | ||
| elementListenerSet = (node: any)[internalEventHandlersKey] = new Set(); | ||
| } | ||
|
|
@@ -246,6 +305,9 @@ export function getFiberFromScopeInstance( | |
| scope: ReactScopeInstance, | ||
| ): null | Fiber { | ||
| if (enableScopeAPI) { | ||
| if (enableInternalInstanceMap) { | ||
| return internalInstanceMap.get(((scope: any): InstanceUnion)) || null; | ||
| } | ||
| return (scope: any)[internalInstanceKey] || null; | ||
| } | ||
| return null; | ||
|
|
@@ -318,6 +380,12 @@ export function clearScrollEndTimer(node: EventTarget): void { | |
| } | ||
|
|
||
| export function isOwnedInstance(node: Node): boolean { | ||
| if (enableInternalInstanceMap) { | ||
| return !!( | ||
| (node: any)[internalHoistableMarker] || | ||
| internalInstanceMap.has((node: any)) | ||
| ); | ||
| } | ||
| return !!( | ||
| (node: any)[internalHoistableMarker] || (node: any)[internalInstanceKey] | ||
| ); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
below you check internalInstanceMap first instead of the node itself, is it intentional to have different precedence in different places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this version I've moved the main instance handle into the internal map but left the container instance handle on the node. So I'm mirroring the existing checks in terms of precedence. This one checks for container first while the other falls back to container.