Move ref type check to receiver #28464

acdlite · 2024-02-27T22:22:33Z

The runtime contains a type check to determine if a user-provided ref is a valid type — a function or object (or a string, when disableStringRefs is off). This currently happens during child reconciliation. This changes it to happen only when the ref is passed to the component that the ref is being attached to.

This is a continuation of the "ref as prop" change — until you actually pass a ref to a HostComponent, class, etc, ref is a normal prop that has no special behavior.

acdlite · 2024-02-27T22:23:47Z

packages/react-reconciler/src/ReactChildFiber.js

-    typeof mixedRef !== 'object'
+    (typeof mixedRef === 'string' ||
+      typeof mixedRef === 'number' ||
+      typeof mixedRef === 'boolean')


This is a subtle change because we used to coerce any non-function, non-object type to a string, but now we explicitly check for only these types. It's only when string refs are not disabled, though.

react-sizebot · 2024-02-27T22:25:34Z

Comparing: fb10a2c...ea0180b

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	+0.04%	176.89 kB	176.95 kB	+0.02%	55.14 kB	55.15 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.05%	179.00 kB	179.10 kB	+0.04%	55.80 kB	55.83 kB
facebook-www/ReactDOM-prod.classic.js	+0.01%	594.04 kB	594.12 kB	+0.01%	104.94 kB	104.95 kB
facebook-www/ReactDOM-prod.modern.js	+0.01%	577.33 kB	577.41 kB	+0.02%	101.96 kB	101.97 kB
test_utils/ReactAllWarnings.js	Deleted	66.40 kB	0.00 kB	Deleted	16.26 kB	0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
test_utils/ReactAllWarnings.js	Deleted	66.40 kB	0.00 kB	Deleted	16.26 kB	0.00 kB

Generated by 🚫 dangerJS against ea0180b

packages/react-reconciler/src/ReactFiberBeginWork.js

eps1lon · 2024-02-27T23:09:49Z

scripts/error-codes/codes.json

@@ -280,7 +280,7 @@
  "281": "Finished root should have a work-in-progress. This error is likely caused by a bug in React. Please file an issue.",
  "282": "If the root does not have an updateQueue, we should have already bailed out. This error is likely caused by a bug in React. Please file an issue.",
  "283": "Element type is invalid. Received a promise that resolves to: %s. Promise elements must resolve to a class or function.",
-  "284": "Expected ref to be a function, a string, an object returned by React.createRef(), or null.",


Can we give this a new error code somehow so that older versions of React decode an error message with their respective ref type?

I did it this way intentionally for log continuity. The new message isn't incorrect for older versions, per se. It doesn't list "string" as a valid type but that's fine because we'd rather then use one of the other ones anyway.

That said I also think it's fine if we make a new error code. Generally I try to avoid creating new ones if the change isn't significant, though.

The runtime contains a type check to determine if a user-provided ref is a valid type — a function or object (or a string, when `disableStringRefs` is off). This currently happens during child reconciliation. This changes it to happen only when the ref is passed to the component that the ref is being attached to. This is a continuation of the "ref as prop" change — until you actually pass a ref to a HostComponent, class, etc, ref is a normal prop that has no special behavior.

String is no longer a valid accepted type. (Except when `disableStringRefs` is off, in the Meta build, but even in that case they are deprecated so we shouldn't list it as a valid type.)

The runtime contains a type check to determine if a user-provided ref is a valid type — a function or object (or a string, when `disableStringRefs` is off). This currently happens during child reconciliation. This changes it to happen only when the ref is passed to the component that the ref is being attached to. This is a continuation of the "ref as prop" change — until you actually pass a ref to a HostComponent, class, etc, ref is a normal prop that has no special behavior. DiffTrain build for [2f8f776](2f8f776)

Frede244

Tutorial ohvcdvvffbvcgcfggggggffghhhhhhggggfffffggg

Frede244 · 2024-02-28T02:33:54Z

packages/react-dom/src/__tests__/refs-test.js

@@ -401,22 +401,20 @@ describe('ref swapping', () => {
        root.render(<div ref={10} />);
      });
    }).rejects.toThrow(
-      'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',


Based on: - #28464 --- This moves the entire string ref implementation out Fiber and into the JSX runtime. The string is converted to a callback ref during element creation. This is a subtle change in behavior, because it will have already been converted to a callback ref if you access element.prop.ref or element.ref. But this is only for Meta, because string refs are disabled entirely in open source. And if it leads to an issue in practice, the solution is to switch to a different ref type, which Meta is going to do regardless.

Based on: - #28464 --- This moves the entire string ref implementation out Fiber and into the JSX runtime. The string is converted to a callback ref during element creation. This is a subtle change in behavior, because it will have already been converted to a callback ref if you access element.prop.ref or element.ref. But this is only for Meta, because string refs are disabled entirely in open source. And if it leads to an issue in practice, the solution is to switch to a different ref type, which Meta is going to do regardless. DiffTrain build for [e3ebcd5](e3ebcd5)

Based on: - #28464 --- This moves the entire string ref implementation out Fiber and into the JSX runtime. The string is converted to a callback ref during element creation. This is a subtle change in behavior, because it will have already been converted to a callback ref if you access element.prop.ref or element.ref. But this is only for Meta, because string refs are disabled entirely in open source. And if it leads to an issue in practice, the solution is to switch to a different ref type, which Meta is going to do regardless.

The runtime contains a type check to determine if a user-provided ref is a valid type — a function or object (or a string, when `disableStringRefs` is off). This currently happens during child reconciliation. This changes it to happen only when the ref is passed to the component that the ref is being attached to. This is a continuation of the "ref as prop" change — until you actually pass a ref to a HostComponent, class, etc, ref is a normal prop that has no special behavior.

Based on: - facebook#28464 --- This moves the entire string ref implementation out Fiber and into the JSX runtime. The string is converted to a callback ref during element creation. This is a subtle change in behavior, because it will have already been converted to a callback ref if you access element.prop.ref or element.ref. But this is only for Meta, because string refs are disabled entirely in open source. And if it leads to an issue in practice, the solution is to switch to a different ref type, which Meta is going to do regardless.

The runtime contains a type check to determine if a user-provided ref is a valid type — a function or object (or a string, when `disableStringRefs` is off). This currently happens during child reconciliation. This changes it to happen only when the ref is passed to the component that the ref is being attached to. This is a continuation of the "ref as prop" change — until you actually pass a ref to a HostComponent, class, etc, ref is a normal prop that has no special behavior. DiffTrain build for commit 2f8f776.

Based on: - #28464 --- This moves the entire string ref implementation out Fiber and into the JSX runtime. The string is converted to a callback ref during element creation. This is a subtle change in behavior, because it will have already been converted to a callback ref if you access element.prop.ref or element.ref. But this is only for Meta, because string refs are disabled entirely in open source. And if it leads to an issue in practice, the solution is to switch to a different ref type, which Meta is going to do regardless. DiffTrain build for commit e3ebcd5.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 27, 2024

acdlite commented Feb 27, 2024

View reviewed changes

acdlite force-pushed the move-ref-type-check branch from 1cd3b85 to 63b3e8c Compare February 27, 2024 22:28

acdlite marked this pull request as ready for review February 27, 2024 22:28

eps1lon reviewed Feb 27, 2024

View reviewed changes

acdlite added 2 commits February 29, 2024 13:35

Remove "string" from invalid ref error message

ea0180b

String is no longer a valid accepted type. (Except when `disableStringRefs` is off, in the Meta build, but even in that case they are deprecated so we shouldn't list it as a valid type.)

acdlite force-pushed the move-ref-type-check branch from 63b3e8c to ea0180b Compare February 29, 2024 18:35

acdlite mentioned this pull request Feb 29, 2024

Move string ref coercion to JSX runtime #28473

Merged

eps1lon approved these changes Feb 29, 2024

View reviewed changes

acdlite merged commit 2f8f776 into facebook:main Mar 1, 2024
38 checks passed

Frede244 reviewed Mar 5, 2024

View reviewed changes

ztanner mentioned this pull request Mar 18, 2024

Update React from 6c3b8dbfe to a4939017f vercel/next.js#63413

Closed

gnoff mentioned this pull request Mar 25, 2024

Update React from 14898b6a9 to e373190fa vercel/next.js#63685

Closed

ztanner mentioned this pull request Apr 2, 2024

Update React from 14898b6a9 to a870b2d54 vercel/next.js#63976

Closed

eps1lon mentioned this pull request Apr 19, 2024

Update React from 14898b6a9 to c3048aab4 vercel/next.js#64798

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move ref type check to receiver #28464

Move ref type check to receiver #28464

acdlite commented Feb 27, 2024

acdlite Feb 27, 2024

react-sizebot commented Feb 27, 2024 •

edited

Loading

eps1lon Feb 27, 2024

acdlite Feb 29, 2024

acdlite Feb 29, 2024

Frede244 left a comment

Frede244 Feb 28, 2024

Move ref type check to receiver #28464

Move ref type check to receiver #28464

Conversation

acdlite commented Feb 27, 2024

acdlite Feb 27, 2024

Choose a reason for hiding this comment

react-sizebot commented Feb 27, 2024 • edited Loading

Critical size changes

Significant size changes

eps1lon Feb 27, 2024

Choose a reason for hiding this comment

acdlite Feb 29, 2024

Choose a reason for hiding this comment

acdlite Feb 29, 2024

Choose a reason for hiding this comment

Frede244 left a comment

Choose a reason for hiding this comment

Frede244 Feb 28, 2024

Choose a reason for hiding this comment

react-sizebot commented Feb 27, 2024 •

edited

Loading