Forward ref by default (#4658)

JoviDeCroock · JoviDeCroock · commit 699b4b50a2f3 · 2025-02-13T17:19:54.000+01:00
* Forward ref by default

* Optimizations
diff --git a/compat/src/forwardRef.js b/compat/src/forwardRef.js
@@ -1,15 +1,5 @@
-import { options } from 'preact';
 import { assign } from './util';
 
-let oldDiffHook = options._diff;
-options._diff = vnode => {
-	if (vnode.type && vnode.type._forwarded && vnode.ref) {
-		vnode.props.ref = vnode.ref;
-		vnode.ref = null;
-	}
-	if (oldDiffHook) oldDiffHook(vnode);
-};
-
 export const REACT_FORWARD_SYMBOL = Symbol.for('react.forward_ref');
 
 /**
@@ -34,7 +24,7 @@ export function forwardRef(fn) {
 	// mobx-react throws.
 	Forwarded.render = Forwarded;
 
-	Forwarded.prototype.isReactComponent = Forwarded._forwarded = true;
+	Forwarded.prototype.isReactComponent = true;
 	Forwarded.displayName = 'ForwardRef(' + (fn.displayName || fn.name) + ')';
 	return Forwarded;
 }
diff --git a/compat/src/internal.d.ts b/compat/src/internal.d.ts
@@ -27,7 +27,6 @@ export interface Component<P = {}, S = {}> extends PreactComponent<P, S> {
 
 export interface FunctionComponent<P = {}> extends PreactFunctionComponent<P> {
 	shouldComponentUpdate?(nextProps: Readonly<P>): boolean;
-	_forwarded?: boolean;
 	_patchedLifecycles?: true;
 }
 
diff --git a/compat/src/memo.js b/compat/src/memo.js
@@ -29,6 +29,5 @@ export function memo(c, comparer) {
 	}
 	Memoed.displayName = 'Memo(' + (c.displayName || c.name) + ')';
 	Memoed.prototype.isReactComponent = true;
-	Memoed._forwarded = true;
 	return Memoed;
 }
diff --git a/compat/src/suspense.js b/compat/src/suspense.js
@@ -268,6 +268,5 @@ export function lazy(loader) {
 	}
 
 	Lazy.displayName = 'Lazy';
-	Lazy._forwarded = true;
 	return Lazy;
 }
diff --git a/compat/test/browser/forwardRef.test.js b/compat/test/browser/forwardRef.test.js
@@ -402,8 +402,7 @@ describe('forwardRef', () => {
 		const Transition = ({ children }) => {
 			const state = useState(0);
 			forceTransition = state[1];
-			expect(children.ref).to.not.be.undefined;
-			if (state[0] === 0) expect(children.props.ref).to.be.undefined;
+			expect(children.props.ref).to.not.be.undefined;
 			return children;
 		};
 
diff --git a/compat/test/browser/memo.test.js b/compat/test/browser/memo.test.js
@@ -72,9 +72,9 @@ describe('memo()', () => {
 
 		let ref = null;
 
-		function Foo() {
+		function Foo(props) {
 			spy();
-			return <h1>Hello World</h1>;
+			return <h1 ref={props.ref}>Hello World</h1>;
 		}
 
 		let Memoized = memo(Foo);
@@ -99,8 +99,7 @@ describe('memo()', () => {
 		update();
 		rerender();
 
-		expect(ref.current).not.to.be.undefined;
-
+		expect(ref.current).to.equal(scratch.firstChild);
 		// TODO: not sure whether this is in-line with react...
 		expect(spy).to.be.calledTwice;
 	});
@@ -175,8 +174,8 @@ describe('memo()', () => {
 
 	it('should pass ref through nested memos', () => {
 		class Foo extends Component {
-			render() {
-				return <h1>Hello World</h1>;
+			render(props) {
+				return <h1 ref={props.ref}>Hello World</h1>;
 			}
 		}
 
@@ -187,7 +186,7 @@ describe('memo()', () => {
 		render(<App ref={ref} />, scratch);
 
 		expect(ref.current).not.to.be.undefined;
-		expect(ref.current).to.be.instanceOf(Foo);
+		expect(ref.current).to.equal(scratch.firstChild);
 	});
 
 	it('should not unnecessarily reorder children #2895', () => {
diff --git a/compat/test/browser/suspense.test.js b/compat/test/browser/suspense.test.js
@@ -271,7 +271,7 @@ describe('suspense', () => {
 	});
 
 	it('lazy should forward refs', () => {
-		const LazyComp = () => <div>Hello from LazyComp</div>;
+		const LazyComp = props => <div ref={props.ref} />;
 		let ref = {};
 
 		/** @type {() => Promise<void>} */
@@ -297,7 +297,7 @@ describe('suspense', () => {
 
 		return resolve().then(() => {
 			rerender();
-			expect(ref.current.constructor).to.equal(LazyComp);
+			expect(ref.current).to.equal(scratch.firstChild);
 		});
 	});
 
@@ -1501,8 +1501,8 @@ describe('suspense', () => {
 
 		/** @type {() => void} */
 		let hide,
-		/** @type {(v) => void} */
-		setValue;
+			/** @type {(v) => void} */
+			setValue;
 
 		class Conditional extends Component {
 			constructor(props) {
@@ -1674,8 +1674,17 @@ describe('suspense', () => {
 
 		// eslint-disable-next-line react/require-render-return
 		class Suspender extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { promise: new Promise(() => {}) };
+				if (typeof props.ref === 'function') {
+					props.ref(this);
+				} else if (props.ref) {
+					props.ref.current = this;
+				}
+			}
 			render() {
-				throw new Promise(() => {});
+				throw this.state.promise;
 			}
 		}
 
diff --git a/debug/src/debug.js b/debug/src/debug.js
@@ -11,7 +11,7 @@ import {
 	getCurrentVNode,
 	getDisplayName
 } from './component-stack';
-import { assign, isNaN } from './util';
+import { isNaN } from './util';
 
 const isWeakMapSupported = typeof WeakMap == 'function';
 
@@ -229,15 +229,12 @@ export function initDebug() {
 				}
 			}
 
-			let values = vnode.props;
-			if (vnode.type._forwarded) {
-				values = assign({}, values);
-				delete values.ref;
-			}
+			/* eslint-disable-next-line */
+			const { ref: _ref, ...props } = vnode.props;
 
 			checkPropTypes(
 				vnode.type.propTypes,
-				values,
+				props,
 				'prop',
 				getDisplayName(vnode),
 				() => getOwnerStack(vnode)
diff --git a/hooks/test/browser/useContext.test.js b/hooks/test/browser/useContext.test.js
@@ -180,6 +180,7 @@ describe('useContext', () => {
 		let provider, subSpy;
 
 		function Comp() {
+			provider = this._vnode._parent._component;
 			const value = useContext(Context);
 			values.push(value);
 			return null;
@@ -188,7 +189,7 @@ describe('useContext', () => {
 		render(<Comp />, scratch);
 
 		render(
-			<Context.Provider ref={p => (provider = p)} value={42}>
+			<Context.Provider value={42}>
 				<Comp />
 			</Context.Provider>,
 			scratch
@@ -212,6 +213,7 @@ describe('useContext', () => {
 		let provider, subSpy;
 
 		function Comp() {
+			provider = this._vnode._parent._component;
 			const value = useContext(Context);
 			values.push(value);
 			return null;
@@ -220,7 +222,7 @@ describe('useContext', () => {
 		render(<Comp />, scratch);
 
 		render(
-			<Context ref={p => (provider = p)} value={42}>
+			<Context value={42}>
 				<Comp />
 			</Context>,
 			scratch
diff --git a/jsx-runtime/src/index.js b/jsx-runtime/src/index.js
@@ -35,7 +35,7 @@ function createVNode(type, props, key, isStaticChildren, __source, __self) {
 		ref,
 		i;
 
-	if ('ref' in normalizedProps) {
+	if ('ref' in normalizedProps && typeof type != 'function') {
 		normalizedProps = {};
 		for (i in props) {
 			if (i == 'ref') {
diff --git a/src/clone-element.js b/src/clone-element.js
@@ -25,7 +25,7 @@ export function cloneElement(vnode, props, children) {
 
 	for (i in props) {
 		if (i == 'key') key = props[i];
-		else if (i == 'ref') ref = props[i];
+		else if (i == 'ref' && typeof vnode.type != 'function') ref = props[i];
 		else if (props[i] === UNDEFINED && defaultProps !== UNDEFINED) {
 			normalizedProps[i] = defaultProps[i];
 		} else {
diff --git a/src/create-element.js b/src/create-element.js
@@ -20,7 +20,7 @@ export function createElement(type, props, children) {
 		i;
 	for (i in props) {
 		if (i == 'key') key = props[i];
-		else if (i == 'ref') ref = props[i];
+		else if (i == 'ref' && typeof type != 'function') ref = props[i];
 		else normalizedProps[i] = props[i];
 	}
 
diff --git a/test/browser/cloneElement.test.js b/test/browser/cloneElement.test.js
@@ -63,8 +63,7 @@ describe('cloneElement', () => {
 	it('should override ref if specified', () => {
 		function a() {}
 		function b() {}
-		function Foo() {}
-		const instance = <Foo ref={a}>hello</Foo>;
+		const instance = <div ref={a}>hello</div>;
 
 		let clone = cloneElement(instance);
 		expect(clone.ref).to.equal(a);
diff --git a/test/browser/keys.test.js b/test/browser/keys.test.js
@@ -17,7 +17,20 @@ describe('keys', () => {
 
 	function createStateful(name) {
 		return class Stateful extends Component {
+			constructor(props) {
+				super(props);
+				if (typeof props.ref === 'function') {
+					props.ref(this);
+				} else if (props.ref) {
+					props.ref.current = this;
+				}
+			}
 			componentDidUpdate() {
+				if (typeof this.props.ref === 'function') {
+					this.props.ref(this);
+				} else if (this.props.ref) {
+					this.props.ref.current = this;
+				}
 				ops.push(`Update ${name}`);
 			}
 			componentDidMount() {
diff --git a/test/browser/lifecycles/componentDidCatch.test.js b/test/browser/lifecycles/componentDidCatch.test.js
@@ -324,7 +324,7 @@ describe('Lifecycle methods', () => {
 		});
 
 		it('should be called when applying a Component ref', () => {
-			const Foo = () => <div />;
+			const Foo = props => <div ref={props.ref} />;
 
 			const ref = value => {
 				if (value) {
diff --git a/test/browser/lifecycles/getDerivedStateFromError.test.js b/test/browser/lifecycles/getDerivedStateFromError.test.js
@@ -325,7 +325,7 @@ describe('Lifecycle methods', () => {
 		});
 
 		it('should be called when applying a Component ref', () => {
-			const Foo = () => <div />;
+			const Foo = props => <div ref={props.ref} />;
 
 			const ref = value => {
 				if (value) {
diff --git a/test/browser/placeholders.test.js b/test/browser/placeholders.test.js
@@ -117,6 +117,11 @@ describe('null placeholders', () => {
 		class Stateful extends Component {
 			constructor(props) {
 				super(props);
+				if (typeof props.ref === 'function') {
+					props.ref(this);
+				} else if (props.ref) {
+					props.ref.current = this;
+				}
 				this.state = { count: 0 };
 			}
 			increment() {
diff --git a/test/browser/refs.test.js b/test/browser/refs.test.js
diff --git a/test/browser/render.test.js b/test/browser/render.test.js

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,6 @@ export interface Component<P = {}, S = {}> extends PreactComponent<P, S> {`
`27`	`27`
`28`	`28`	`export interface FunctionComponent<P = {}> extends PreactFunctionComponent<P> {`
`29`	`29`	`shouldComponentUpdate?(nextProps: Readonly<P>): boolean;`
`30`		`- _forwarded?: boolean;`
`31`	`30`	`_patchedLifecycles?: true;`
`32`	`31`	`}`
`33`	`32`
Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,5 @@ export function memo(c, comparer) {`
`29`	`29`	`}`
`30`	`30`	`Memoed.displayName = 'Memo(' + (c.displayName \|\| c.name) + ')';`
`31`	`31`	`Memoed.prototype.isReactComponent = true;`
`32`		`- Memoed._forwarded = true;`
`33`	`32`	`return Memoed;`
`34`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -268,6 +268,5 @@ export function lazy(loader) {`
`268`	`268`	`}`
`269`	`269`
`270`	`270`	`Lazy.displayName = 'Lazy';`
`271`		`- Lazy._forwarded = true;`
`272`	`271`	`return Lazy;`
`273`	`272`	`}`