Skip to content

Commit e574453

Browse files
sebmarkbagesebmarkbage
committed
Move createElement/JSX Warnings into the Renderer (#29088)
This is necessary to simplify the component stack handling to make way for owner stacks. It also solves some hacks that we used to have but don't quite make sense. It also solves the problem where things like key warnings get silenced in RSC because they get deduped. It also surfaces areas where we were missing key warnings to begin with. Almost every type of warning is issued from the renderer. React Elements are really not anything special themselves. They're just lazily invoked functions and its really the renderer that determines there semantics. We have three types of warnings that previously fired in JSX/createElement: - Fragment props validation. - Type validation. - Key warning. It's nice to be able to do some validation in the JSX/createElement because it has a more specific stack frame at the callsite. However, that's the case for every type of component and validation. That's the whole point of enableOwnerStacks. It's also not sufficient to do it in JSX/createElement so we also have validation in the renderers too. So this validation is really just an eager validation but also happens again later. The problem with these is that we don't really know what types are valid until we get to the renderer. Additionally, by placing it in the isomorphic code it becomes harder to do deduping of warnings in a way that makes sense for that renderer. It also means we can't reuse logic for managing stacks etc. Fragment props validation really should just be part of the renderer like any other component type. This also matters once we add Fragment refs and other fragment features. So I moved this into Fiber. However, since some Fragments don't have Fibers, I do the validation in ChildFiber instead of beginWork where it would normally happen. For `type` validation we already do validation when rendering. By leaving it to the renderer we don't have to hard code an extra list. This list also varies by context. E.g. class components aren't allowed in RSC but client references are but we don't have an isomorphic way to identify client references because they're defined by the host config so the current logic is flawed anyway. I kept the early validation for now without the `enableOwnerStacks` since it does provide a nicer stack frame but with that flag on it'll be handled with nice stacks anyway. I normalized some of the errors to ensure tests pass. For `key` validation it's the same principle. The mechanism for the heuristic is still the same - if it passes statically through a parent JSX/createElement call then it's considered validated. We already did print the error later from the renderer so this also disables the early log in the `enableOwnerStacks` flag. I also added logging to Fizz so that key warnings can print in SSR logs. Flight is a bit more complex. For elements that end up on the client we just pass the `validated` flag along to the client and let the client renderer print the error once rendered. For server components we log the error from Flight with the server component as the owner on the stack which will allow us to print the right stack for context. The factoring of this is a little tricky because we only want to warn if it's in an array parent but we want to log the error later to get the right debug info. Fiber/Fizz has a similar factoring problem that causes us to create a fake Fiber for the owner which means the logs won't be associated with the right place in DevTools. DiffTrain build for [84239da](84239da)
1 parent 2de2c20 commit e574453

39 files changed

+1710
-569
lines changed

‎compiled/facebook-www/JSXDEVRuntime-dev.classic.js

Lines changed: 35 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ function printWarning(level, format, args) {
9191

9292
var ReactSharedInternals = React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE; // Defensive in case this is fired before React is initialized.
9393

94-
if (ReactSharedInternals != null) {
95-
var stack = ReactSharedInternals.getStackAddendum();
94+
if (ReactSharedInternals != null && ReactSharedInternals.getCurrentStack) {
95+
var stack = ReactSharedInternals.getCurrentStack();
9696

9797
if (stack !== '') {
9898
format += '%s';
@@ -318,7 +318,9 @@ function checkPropStringCoercion(value, propName) {
318318
}
319319
}
320320

321-
var REACT_CLIENT_REFERENCE$1 = Symbol.for('react.client.reference');
321+
var REACT_CLIENT_REFERENCE$1 = Symbol.for('react.client.reference'); // This function is deprecated. Don't use. Only the renderer knows what a valid type is.
322+
// TODO: Delete this when enableOwnerStacks ships.
323+
322324
function isValidElementType(type) {
323325
if (typeof type === 'string' || typeof type === 'function') {
324326
return true;
@@ -716,7 +718,8 @@ function describeFunctionComponentFrame(fn) {
716718
function shouldConstruct(Component) {
717719
var prototype = Component.prototype;
718720
return !!(prototype && prototype.isReactComponent);
719-
}
721+
} // TODO: Delete this once the key warning no longer uses it. I.e. when enableOwnerStacks ship.
722+
720723

721724
function describeUnknownElementTypeFrameInDEV(type) {
722725

@@ -1154,7 +1157,7 @@ function ReactElement(type, key, _ref, self, source, owner, props, debugStack, d
11541157
configurable: false,
11551158
enumerable: false,
11561159
writable: true,
1157-
value: false
1160+
value: 0
11581161
}); // debugInfo contains Server Component debug information.
11591162

11601163
Object.defineProperty(element, '_debugInfo', {
@@ -1346,29 +1349,7 @@ function jsxDEV$1(type, config, maybeKey, isStaticChildren, source, self) {
13461349
}
13471350
}
13481351

1349-
var element = ReactElement(type, key, ref, self, source, getOwner(), props);
1350-
1351-
if (type === REACT_FRAGMENT_TYPE) {
1352-
validateFragmentProps(element);
1353-
}
1354-
1355-
return element;
1356-
}
1357-
}
1358-
1359-
function getDeclarationErrorAddendum() {
1360-
{
1361-
var owner = getOwner();
1362-
1363-
if (owner) {
1364-
var name = getComponentNameFromType(owner.type);
1365-
1366-
if (name) {
1367-
return '\n\nCheck the render method of `' + name + '`.';
1368-
}
1369-
}
1370-
1371-
return '';
1352+
return ReactElement(type, key, ref, self, source, getOwner(), props);
13721353
}
13731354
}
13741355
/**
@@ -1381,7 +1362,6 @@ function getDeclarationErrorAddendum() {
13811362
* @param {*} parentType node's parent's type.
13821363
*/
13831364

1384-
13851365
function validateChildKeys(node, parentType) {
13861366
{
13871367
if (typeof node !== 'object' || !node) {
@@ -1399,7 +1379,7 @@ function validateChildKeys(node, parentType) {
13991379
} else if (isValidElement(node)) {
14001380
// This element was passed in a valid location.
14011381
if (node._store) {
1402-
node._store.validated = true;
1382+
node._store.validated = 1;
14031383
}
14041384
} else {
14051385
var iteratorFn = getIteratorFn(node);
@@ -1450,12 +1430,13 @@ var ownerHasKeyUseWarning = {};
14501430
*/
14511431

14521432
function validateExplicitKey(element, parentType) {
1433+
14531434
{
14541435
if (!element._store || element._store.validated || element.key != null) {
14551436
return;
14561437
}
14571438

1458-
element._store.validated = true;
1439+
element._store.validated = 1;
14591440
var currentComponentErrorInfo = getCurrentComponentErrorInfo(parentType);
14601441

14611442
if (ownerHasKeyUseWarning[currentComponentErrorInfo]) {
@@ -1481,28 +1462,37 @@ function validateExplicitKey(element, parentType) {
14811462
childOwner = " It was passed a child from " + ownerName + ".";
14821463
}
14831464

1484-
setCurrentlyValidatingElement(element);
1465+
var prevGetCurrentStack = ReactSharedInternals.getCurrentStack;
14851466

1486-
error('Each child in a list should have a unique "key" prop.' + '%s%s See https://react.dev/link/warning-keys for more information.', currentComponentErrorInfo, childOwner);
1467+
ReactSharedInternals.getCurrentStack = function () {
14871468

1488-
setCurrentlyValidatingElement(null);
1489-
}
1490-
}
1469+
var stack = describeUnknownElementTypeFrameInDEV(element.type); // Delegate to the injected renderer-specific implementation
14911470

1492-
function setCurrentlyValidatingElement(element) {
1493-
{
1494-
if (element) {
1495-
var stack = describeUnknownElementTypeFrameInDEV(element.type);
1496-
ReactSharedInternals.setExtraStackFrame(stack);
1497-
} else {
1498-
ReactSharedInternals.setExtraStackFrame(null);
1499-
}
1471+
if (prevGetCurrentStack) {
1472+
stack += prevGetCurrentStack() || '';
1473+
}
1474+
1475+
return stack;
1476+
};
1477+
1478+
error('Each child in a list should have a unique "key" prop.' + '%s%s See https://react.dev/link/warning-keys for more information.', currentComponentErrorInfo, childOwner);
1479+
1480+
ReactSharedInternals.getCurrentStack = prevGetCurrentStack;
15001481
}
15011482
}
15021483

15031484
function getCurrentComponentErrorInfo(parentType) {
15041485
{
1505-
var info = getDeclarationErrorAddendum();
1486+
var info = '';
1487+
var owner = getOwner();
1488+
1489+
if (owner) {
1490+
var name = getComponentNameFromType(owner.type);
1491+
1492+
if (name) {
1493+
info = '\n\nCheck the render method of `' + name + '`.';
1494+
}
1495+
}
15061496

15071497
if (!info) {
15081498
var parentName = getComponentNameFromType(parentType);
@@ -1515,39 +1505,6 @@ function getCurrentComponentErrorInfo(parentType) {
15151505
return info;
15161506
}
15171507
}
1518-
/**
1519-
* Given a fragment, validate that it can only be provided with fragment props
1520-
* @param {ReactElement} fragment
1521-
*/
1522-
1523-
1524-
function validateFragmentProps(fragment) {
1525-
// TODO: Move this to render phase instead of at element creation.
1526-
{
1527-
var keys = Object.keys(fragment.props);
1528-
1529-
for (var i = 0; i < keys.length; i++) {
1530-
var key = keys[i];
1531-
1532-
if (key !== 'children' && key !== 'key') {
1533-
setCurrentlyValidatingElement(fragment);
1534-
1535-
error('Invalid prop `%s` supplied to `React.Fragment`. ' + 'React.Fragment can only have `key` and `children` props.', key);
1536-
1537-
setCurrentlyValidatingElement(null);
1538-
break;
1539-
}
1540-
}
1541-
1542-
if (!enableRefAsProp && fragment.ref !== null) {
1543-
setCurrentlyValidatingElement(fragment);
1544-
1545-
error('Invalid attribute `ref` supplied to `React.Fragment`.');
1546-
1547-
setCurrentlyValidatingElement(null);
1548-
}
1549-
}
1550-
}
15511508

15521509
function coerceStringRef(mixedRef, owner, type) {
15531510

‎compiled/facebook-www/JSXDEVRuntime-dev.modern.js

Lines changed: 35 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ function printWarning(level, format, args) {
9191

9292
var ReactSharedInternals = React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE; // Defensive in case this is fired before React is initialized.
9393

94-
if (ReactSharedInternals != null) {
95-
var stack = ReactSharedInternals.getStackAddendum();
94+
if (ReactSharedInternals != null && ReactSharedInternals.getCurrentStack) {
95+
var stack = ReactSharedInternals.getCurrentStack();
9696

9797
if (stack !== '') {
9898
format += '%s';
@@ -318,7 +318,9 @@ function checkPropStringCoercion(value, propName) {
318318
}
319319
}
320320

321-
var REACT_CLIENT_REFERENCE$1 = Symbol.for('react.client.reference');
321+
var REACT_CLIENT_REFERENCE$1 = Symbol.for('react.client.reference'); // This function is deprecated. Don't use. Only the renderer knows what a valid type is.
322+
// TODO: Delete this when enableOwnerStacks ships.
323+
322324
function isValidElementType(type) {
323325
if (typeof type === 'string' || typeof type === 'function') {
324326
return true;
@@ -716,7 +718,8 @@ function describeFunctionComponentFrame(fn) {
716718
function shouldConstruct(Component) {
717719
var prototype = Component.prototype;
718720
return !!(prototype && prototype.isReactComponent);
719-
}
721+
} // TODO: Delete this once the key warning no longer uses it. I.e. when enableOwnerStacks ship.
722+
720723

721724
function describeUnknownElementTypeFrameInDEV(type) {
722725

@@ -1157,7 +1160,7 @@ function ReactElement(type, key, _ref, self, source, owner, props, debugStack, d
11571160
configurable: false,
11581161
enumerable: false,
11591162
writable: true,
1160-
value: false
1163+
value: 0
11611164
}); // debugInfo contains Server Component debug information.
11621165

11631166
Object.defineProperty(element, '_debugInfo', {
@@ -1349,29 +1352,7 @@ function jsxDEV$1(type, config, maybeKey, isStaticChildren, source, self) {
13491352
}
13501353
}
13511354

1352-
var element = ReactElement(type, key, ref, self, source, getOwner(), props);
1353-
1354-
if (type === REACT_FRAGMENT_TYPE) {
1355-
validateFragmentProps(element);
1356-
}
1357-
1358-
return element;
1359-
}
1360-
}
1361-
1362-
function getDeclarationErrorAddendum() {
1363-
{
1364-
var owner = getOwner();
1365-
1366-
if (owner) {
1367-
var name = getComponentNameFromType(owner.type);
1368-
1369-
if (name) {
1370-
return '\n\nCheck the render method of `' + name + '`.';
1371-
}
1372-
}
1373-
1374-
return '';
1355+
return ReactElement(type, key, ref, self, source, getOwner(), props);
13751356
}
13761357
}
13771358
/**
@@ -1384,7 +1365,6 @@ function getDeclarationErrorAddendum() {
13841365
* @param {*} parentType node's parent's type.
13851366
*/
13861367

1387-
13881368
function validateChildKeys(node, parentType) {
13891369
{
13901370
if (typeof node !== 'object' || !node) {
@@ -1402,7 +1382,7 @@ function validateChildKeys(node, parentType) {
14021382
} else if (isValidElement(node)) {
14031383
// This element was passed in a valid location.
14041384
if (node._store) {
1405-
node._store.validated = true;
1385+
node._store.validated = 1;
14061386
}
14071387
} else {
14081388
var iteratorFn = getIteratorFn(node);
@@ -1453,12 +1433,13 @@ var ownerHasKeyUseWarning = {};
14531433
*/
14541434

14551435
function validateExplicitKey(element, parentType) {
1436+
14561437
{
14571438
if (!element._store || element._store.validated || element.key != null) {
14581439
return;
14591440
}
14601441

1461-
element._store.validated = true;
1442+
element._store.validated = 1;
14621443
var currentComponentErrorInfo = getCurrentComponentErrorInfo(parentType);
14631444

14641445
if (ownerHasKeyUseWarning[currentComponentErrorInfo]) {
@@ -1484,28 +1465,37 @@ function validateExplicitKey(element, parentType) {
14841465
childOwner = " It was passed a child from " + ownerName + ".";
14851466
}
14861467

1487-
setCurrentlyValidatingElement(element);
1468+
var prevGetCurrentStack = ReactSharedInternals.getCurrentStack;
14881469

1489-
error('Each child in a list should have a unique "key" prop.' + '%s%s See https://react.dev/link/warning-keys for more information.', currentComponentErrorInfo, childOwner);
1470+
ReactSharedInternals.getCurrentStack = function () {
14901471

1491-
setCurrentlyValidatingElement(null);
1492-
}
1493-
}
1472+
var stack = describeUnknownElementTypeFrameInDEV(element.type); // Delegate to the injected renderer-specific implementation
14941473

1495-
function setCurrentlyValidatingElement(element) {
1496-
{
1497-
if (element) {
1498-
var stack = describeUnknownElementTypeFrameInDEV(element.type);
1499-
ReactSharedInternals.setExtraStackFrame(stack);
1500-
} else {
1501-
ReactSharedInternals.setExtraStackFrame(null);
1502-
}
1474+
if (prevGetCurrentStack) {
1475+
stack += prevGetCurrentStack() || '';
1476+
}
1477+
1478+
return stack;
1479+
};
1480+
1481+
error('Each child in a list should have a unique "key" prop.' + '%s%s See https://react.dev/link/warning-keys for more information.', currentComponentErrorInfo, childOwner);
1482+
1483+
ReactSharedInternals.getCurrentStack = prevGetCurrentStack;
15031484
}
15041485
}
15051486

15061487
function getCurrentComponentErrorInfo(parentType) {
15071488
{
1508-
var info = getDeclarationErrorAddendum();
1489+
var info = '';
1490+
var owner = getOwner();
1491+
1492+
if (owner) {
1493+
var name = getComponentNameFromType(owner.type);
1494+
1495+
if (name) {
1496+
info = '\n\nCheck the render method of `' + name + '`.';
1497+
}
1498+
}
15091499

15101500
if (!info) {
15111501
var parentName = getComponentNameFromType(parentType);
@@ -1518,39 +1508,6 @@ function getCurrentComponentErrorInfo(parentType) {
15181508
return info;
15191509
}
15201510
}
1521-
/**
1522-
* Given a fragment, validate that it can only be provided with fragment props
1523-
* @param {ReactElement} fragment
1524-
*/
1525-
1526-
1527-
function validateFragmentProps(fragment) {
1528-
// TODO: Move this to render phase instead of at element creation.
1529-
{
1530-
var keys = Object.keys(fragment.props);
1531-
1532-
for (var i = 0; i < keys.length; i++) {
1533-
var key = keys[i];
1534-
1535-
if (key !== 'children' && key !== 'key') {
1536-
setCurrentlyValidatingElement(fragment);
1537-
1538-
error('Invalid prop `%s` supplied to `React.Fragment`. ' + 'React.Fragment can only have `key` and `children` props.', key);
1539-
1540-
setCurrentlyValidatingElement(null);
1541-
break;
1542-
}
1543-
}
1544-
1545-
if (!enableRefAsProp && fragment.ref !== null) {
1546-
setCurrentlyValidatingElement(fragment);
1547-
1548-
error('Invalid attribute `ref` supplied to `React.Fragment`.');
1549-
1550-
setCurrentlyValidatingElement(null);
1551-
}
1552-
}
1553-
}
15541511

15551512
function coerceStringRef(mixedRef, owner, type) {
15561513

‎compiled/facebook-www/REVISION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2e540e22b2b4038a278b2875306976b016fb31a9
1+
84239da896fd7395a667ab1e7ef1ef338a32de8f

0 commit comments

Comments
 (0)