Skip to content

Commit

Permalink
[Fiber][Float] Error when a host fiber changes "flavor" (#29693)
Browse files Browse the repository at this point in the history
Host Components can exist as four semantic types

1. regular Components (Vanilla obv)
2. singleton Components
2. hoistable components
3. resources

Each of these component types have their own rules related to mounting
and reconciliation however they are not direclty modeled as their own
unique fiber type. This is partly for code size but also because
reconciling the inner type of these components would be in a very hot
path in fiber creation and reconciliation and it's just not practical to
do this logic check here.

Right now we have three Fiber types used to implement these 4 concepts
but we probably need to reconsider the model and think of Host
Components as a single fiber type with an inner implementation. Once we
do this we can regularize things like transitioning between a resource
and a regular component or a singleton and a hoistable instance. The
cases where these transitions happen today aren't particularly common
but they can be observed and currently the handling of these transitions
is incomplete at best and buggy at worst. The most egregious case is the
link type. This can be a regular component (stylesheet without
precedence) a hoistable component (non stylesheet link tags) or a
resource (stylesheet with a precedence) and if you have a single jsx
slot that tries to reconcile transitions between these types it just
doesn't work well.

This commit adds an error for when a Hoistable goes from Instance to
Resource. Currently this is only possible for `<link>` elements going to
and from stylesheets with precedence. Hopefully we'll be able to remove
this error and implement as an inner type before we encounter new
categories for the Hoistable types

detecting type shifting to and from regular components is harder to do
efficiently because we don't want to reevaluate the type on every update
for host components which is currently not required and would add
overhead to a very hot path

singletons can't really type shift in their one practical implementation
(DOM) so they are only a problem in theroy not practice

DiffTrain build for [47d0c30](47d0c30)
  • Loading branch information
gnoff committed Jun 3, 2024
1 parent 9f1a9c5 commit a405877
Show file tree
Hide file tree
Showing 34 changed files with 1,647 additions and 1,357 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
b421783110fb20f139adf4c4f9a8911dc63f9c68
47d0c30246134ad9ce04abdcf0977cf2d49d00ce
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION_TRANSFORMS
Original file line number Diff line number Diff line change
@@ -1 +1 @@
b421783110fb20f139adf4c4f9a8911dc63f9c68
47d0c30246134ad9ce04abdcf0977cf2d49d00ce
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if (
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = '19.0.0-www-classic-b421783110-20240603';
var ReactVersion = '19.0.0-www-classic-47d0c30246-20240603';

// Re-export dynamic flags from the www version.
var dynamicFeatureFlags = require('ReactFeatureFlags');
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if (
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = '19.0.0-www-modern-b421783110-20240603';
var ReactVersion = '19.0.0-www-modern-47d0c30246-20240603';

// Re-export dynamic flags from the www version.
var dynamicFeatureFlags = require('ReactFeatureFlags');
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-classic-b421783110-20240603";
exports.version = "19.0.0-www-classic-47d0c30246-20240603";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-modern-b421783110-20240603";
exports.version = "19.0.0-www-modern-47d0c30246-20240603";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-classic-b421783110-20240603";
exports.version = "19.0.0-www-classic-47d0c30246-20240603";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactSharedInternals.H.useTransition();
};
exports.version = "19.0.0-www-modern-b421783110-20240603";
exports.version = "19.0.0-www-modern-47d0c30246-20240603";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = '19.0.0-www-classic-b421783110-20240603';
var ReactVersion = '19.0.0-www-classic-47d0c30246-20240603';

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = '19.0.0-www-modern-b421783110-20240603';
var ReactVersion = '19.0.0-www-modern-47d0c30246-20240603';

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down
4 changes: 2 additions & 2 deletions compiled/facebook-www/ReactART-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -10645,7 +10645,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "19.0.0-www-classic-b421783110-20240603",
version: "19.0.0-www-classic-47d0c30246-20240603",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1370 = {
Expand Down Expand Up @@ -10676,7 +10676,7 @@ var internals$jscomp$inline_1370 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-www-classic-b421783110-20240603"
reconcilerVersion: "19.0.0-www-classic-47d0c30246-20240603"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1371 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
4 changes: 2 additions & 2 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -10120,7 +10120,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "19.0.0-www-modern-b421783110-20240603",
version: "19.0.0-www-modern-47d0c30246-20240603",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1356 = {
Expand Down Expand Up @@ -10151,7 +10151,7 @@ var internals$jscomp$inline_1356 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "19.0.0-www-modern-b421783110-20240603"
reconcilerVersion: "19.0.0-www-modern-47d0c30246-20240603"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1357 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
125 changes: 97 additions & 28 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -18306,14 +18306,22 @@ function updateHostComponent$1(current, workInProgress, renderLanes) {

function updateHostHoistable(current, workInProgress, renderLanes) {
markRef(current, workInProgress);
var currentProps = current === null ? null : current.memoizedProps;
var resource = workInProgress.memoizedState = getResource(workInProgress.type, currentProps, workInProgress.pendingProps);

if (current === null) {
if (!getIsHydrating() && resource === null) {
// This is not a Resource Hoistable and we aren't hydrating so we construct the instance.
workInProgress.stateNode = createHoistableInstance(workInProgress.type, workInProgress.pendingProps, getRootHostContainer(), workInProgress);
var resource = getResource(workInProgress.type, null, workInProgress.pendingProps, null);

if (resource) {
workInProgress.memoizedState = resource;
} else {
if (!getIsHydrating()) {
// This is not a Resource Hoistable and we aren't hydrating so we construct the instance.
workInProgress.stateNode = createHoistableInstance(workInProgress.type, workInProgress.pendingProps, getRootHostContainer(), workInProgress);
}
}
} else {
// Get Resource may or may not return a resource. either way we stash the result
// on memoized state.
workInProgress.memoizedState = getResource(workInProgress.type, current.memoizedProps, workInProgress.pendingProps, current.memoizedState);
} // Resources never have reconciler managed children. It is possible for
// the host implementation of getResource to consider children in the
// resource construction but they will otherwise be discarded. In practice
Expand Down Expand Up @@ -21835,29 +21843,28 @@ function completeWork(current, workInProgress, renderLanes) {
return null;
}
} else {
// We are updating.
var currentResource = current.memoizedState;

if (nextResource !== currentResource) {
// We are transitioning to, from, or between Hoistable Resources
// and require an update
markUpdate(workInProgress);
}

if (nextResource !== null) {
// This is a Hoistable Resource
// This must come at the very end of the complete phase.
bubbleProperties(workInProgress);
// This is an update.
if (nextResource) {
// This is a Resource
if (nextResource !== current.memoizedState) {
// we have a new Resource. we need to update
markUpdate(workInProgress); // This must come at the very end of the complete phase.

bubbleProperties(workInProgress); // This must come at the very end of the complete phase, because it might
// throw to suspend, and if the resource immediately loads, the work loop
// will resume rendering as if the work-in-progress completed. So it must
// fully complete.

if (nextResource === currentResource) {
workInProgress.flags &= ~MaySuspendCommit;
} else {
preloadResourceAndSuspendIfNeeded(workInProgress, nextResource);
return null;
} else {
// This must come at the very end of the complete phase.
bubbleProperties(workInProgress);
workInProgress.flags &= ~MaySuspendCommit;
return null;
}

return null;
} else {
// This is a Hoistable Instance
// This is an Instance
// We may have props to update on the Hoistable instance.
{
var oldProps = current.memoizedProps;
Expand Down Expand Up @@ -31083,7 +31090,7 @@ identifierPrefix, onUncaughtError, onCaughtError, onRecoverableError, transition
return root;
}

var ReactVersion = '19.0.0-www-classic-b421783110-20240603';
var ReactVersion = '19.0.0-www-classic-47d0c30246-20240603';

function createPortal$1(children, containerInfo, // TODO: figure out the API for cross-renderer implementation.
implementation) {
Expand Down Expand Up @@ -39307,7 +39314,7 @@ function preinitModuleScript(src, options) {
} // This function is called in begin work and we should always have a currentDocument set


function getResource(type, currentProps, pendingProps) {
function getResource(type, currentProps, pendingProps, currentResource) {
var resourceRoot = getCurrentResourceRoot();

if (!resourceRoot) {
Expand Down Expand Up @@ -39380,10 +39387,34 @@ function getResource(type, currentProps, pendingProps) {
}
}

if (currentProps && currentResource === null) {
// This node was previously an Instance type and is becoming a Resource type
// For now we error because we don't support flavor changes
var diff = '';

{
diff = "\n\n - " + describeLinkForResourceErrorDEV(currentProps) + "\n + " + describeLinkForResourceErrorDEV(pendingProps);
}

throw new Error('Expected <link> not to update to be updated to a stylehsheet with precedence.' + ' Check the `rel`, `href`, and `precedence` props of this component.' + ' Alternatively, check whether two different <link> components render in the same slot or share the same key.' + diff);
}

return _resource;
}
} else {
if (currentProps && currentResource !== null) {
// This node was previously a Resource type and is becoming an Instance type
// For now we error because we don't support flavor changes
var _diff = '';

return null;
{
_diff = "\n\n - " + describeLinkForResourceErrorDEV(currentProps) + "\n + " + describeLinkForResourceErrorDEV(pendingProps);
}

throw new Error('Expected stylesheet with precedence to not be updated to a different kind of <link>.' + ' Check the `rel`, `href`, and `precedence` props of this component.' + ' Alternatively, check whether two different <link> components render in the same slot or share the same key.' + _diff);
}

return null;
}
}

case 'script':
Expand Down Expand Up @@ -39426,6 +39457,44 @@ function getResource(type, currentProps, pendingProps) {
}
}

function describeLinkForResourceErrorDEV(props) {
{
var describedProps = 0;
var description = '<link';

if (typeof props.rel === 'string') {
describedProps++;
description += " rel=\"" + props.rel + "\"";
} else if (hasOwnProperty.call(props, 'rel')) {
describedProps++;
description += " rel=\"" + (props.rel === null ? 'null' : 'invalid type ' + typeof props.rel) + "\"";
}

if (typeof props.href === 'string') {
describedProps++;
description += " href=\"" + props.href + "\"";
} else if (hasOwnProperty.call(props, 'href')) {
describedProps++;
description += " href=\"" + (props.href === null ? 'null' : 'invalid type ' + typeof props.href) + "\"";
}

if (typeof props.precedence === 'string') {
describedProps++;
description += " precedence=\"" + props.precedence + "\"";
} else if (hasOwnProperty.call(props, 'precedence')) {
describedProps++;
description += " precedence={" + (props.precedence === null ? 'null' : 'invalid type ' + typeof props.precedence) + "}";
}

if (Object.getOwnPropertyNames(props).length > describedProps) {
description += ' ...';
}

description += ' />';
return description;
}
}

function styleTagPropsFromRawProps(rawProps) {
return assign({}, rawProps, {
'data-href': rawProps.href,
Expand Down
Loading

0 comments on commit a405877

Please sign in to comment.