Move default props to compat (#3366)

* add default props to compat and golf ref/key handling * fix tests * add changeset * remove unneeded test * make abstract class * fix types * [types] Inject defaultProps into preact.Component * attempt to use ambient module for decl merging * Update compat/src/render.js Co-authored-by: Jason Miller <[email protected]> Co-authored-by: Jason Miller <[email protected]>
preactjs · Dec 9, 2021 · 5a6f036 · 5a6f036
1 parent 8bea204
commit 5a6f036
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 102 deletions.
diff --git a/.changeset/little-spies-crash.md b/.changeset/little-spies-crash.md
@@ -0,0 +1,5 @@
+---
+'preact': major
+---
+
+Move `defaultProps` handling to preact/compat
diff --git a/compat/src/index.d.ts b/compat/src/index.d.ts
@@ -4,6 +4,17 @@ import { JSXInternal } from '../../src/jsx';
 import * as _Suspense from './suspense';
 import * as _SuspenseList from './suspense-list';
 
+declare module 'preact' {
+	export namespace preact {
+		interface FunctionComponent<P = {}> {
+			defaultProps?: Partial<P>;
+		}
+		interface ComponentClass<P = {}, S = {}> {
+			defaultProps?: Partial<P>;
+		}
+	}
+}
+
 // export default React;
 export = React;
 export as namespace React;

diff --git a/compat/src/render.js b/compat/src/render.js
@@ -210,6 +210,12 @@ options.vnode = vnode => {
 			if (props.className != null) normalizedProps.class = props.className;
 			Object.defineProperty(normalizedProps, 'className', classNameDescriptor);
 		}
+	} else if (typeof type === 'function' && type.defaultProps) {
+		for (i in type.defaultProps) {
+			if (normalizedProps[i] === undefined) {
+				normalizedProps[i] = type.defaultProps[i];
+			}
+		}
 	}
 
 	vnode.$$typeof = REACT_ELEMENT_TYPE;

diff --git a/compat/test/browser/component.test.js b/compat/test/browser/component.test.js
@@ -1,6 +1,6 @@
 import { setupRerender } from 'preact/test-utils';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
-import React, { createElement } from 'preact/compat';
+import React, { createElement, Component } from 'preact/compat';
 
 describe('components', () => {
 	/** @type {HTMLDivElement} */
@@ -240,4 +240,89 @@ describe('components', () => {
 			expect(Page.prototype.UNSAFE_componentWillMount).to.have.been.called;
 		});
 	});
+
+	describe('defaultProps', () => {
+		it('should apply default props on initial render', () => {
+			class WithDefaultProps extends Component {
+				constructor(props, context) {
+					super(props, context);
+					expect(props).to.be.deep.equal({
+						fieldA: 1,
+						fieldB: 2,
+						fieldC: 1,
+						fieldD: 2
+					});
+				}
+				render() {
+					return <div />;
+				}
+			}
+			WithDefaultProps.defaultProps = { fieldC: 1, fieldD: 1 };
+			React.render(
+				<WithDefaultProps fieldA={1} fieldB={2} fieldD={2} />,
+				scratch
+			);
+		});
+
+		it('should apply default props on rerender', () => {
+			let doRender;
+			class Outer extends Component {
+				constructor() {
+					super();
+					this.state = { i: 1 };
+				}
+				componentDidMount() {
+					doRender = () => this.setState({ i: 2 });
+				}
+				render(props, { i }) {
+					return <WithDefaultProps fieldA={1} fieldB={i} fieldD={i} />;
+				}
+			}
+			class WithDefaultProps extends Component {
+				constructor(props, context) {
+					super(props, context);
+					this.ctor(props, context);
+				}
+				ctor() {}
+				componentWillReceiveProps() {}
+				render() {
+					return <div />;
+				}
+			}
+			WithDefaultProps.defaultProps = { fieldC: 1, fieldD: 1 };
+
+			let proto = WithDefaultProps.prototype;
+			sinon.spy(proto, 'ctor');
+			sinon.spy(proto, 'componentWillReceiveProps');
+			sinon.spy(proto, 'render');
+
+			React.render(<Outer />, scratch);
+			doRender();
+
+			const PROPS1 = {
+				fieldA: 1,
+				fieldB: 1,
+				fieldC: 1,
+				fieldD: 1
+			};
+
+			const PROPS2 = {
+				fieldA: 1,
+				fieldB: 2,
+				fieldC: 1,
+				fieldD: 2
+			};
+
+			expect(proto.ctor).to.have.been.calledWithMatch(PROPS1);
+			expect(proto.render).to.have.been.calledWithMatch(PROPS1);
+
+			rerender();
+
+			// expect(proto.ctor).to.have.been.calledWith(PROPS2);
+			expect(proto.componentWillReceiveProps).to.have.been.calledWithMatch(
+				PROPS2
+			);
+			expect(proto.render).to.have.been.calledWithMatch(PROPS2);
+		});
+	});
 });
diff --git a/src/clone-element.js b/src/clone-element.js
@@ -12,6 +12,7 @@ export function cloneElement(vnode, props, children) {
 		key,
 		ref,
 		i;
+
 	for (i in props) {
 		if (i == 'key') key = props[i];
 		else if (i == 'ref') ref = props[i];

diff --git a/src/create-element.js b/src/create-element.js
@@ -34,16 +34,6 @@ export function createElement(type, props, children) {
 		normalizedProps.children = children;
 	}
 
-	// If a Component VNode, check for and apply defaultProps
-	// Note: type may be undefined in development, must never error here.
-	if (typeof type == 'function' && type.defaultProps != null) {
-		for (i in type.defaultProps) {
-			if (normalizedProps[i] === UNDEFINED) {
-				normalizedProps[i] = type.defaultProps[i];
-			}
-		}
-	}
-
 	return createVNode(type, normalizedProps, key, ref, 0);
 }
 

diff --git a/src/index.d.ts b/src/index.d.ts
@@ -85,14 +85,12 @@ export type ComponentProps<
 export interface FunctionComponent<P = {}> {
 	(props: RenderableProps<P>, context?: any): VNode<any> | null;
 	displayName?: string;
-	defaultProps?: Partial<P>;
 }
 export interface FunctionalComponent<P = {}> extends FunctionComponent<P> {}
 
 export interface ComponentClass<P = {}, S = {}> {
 	new (props: P, context?: any): Component<P, S>;
 	displayName?: string;
-	defaultProps?: Partial<P>;
 	contextType?: Context<any>;
 	getDerivedStateFromProps?(
 		props: Readonly<P>,
@@ -137,7 +135,6 @@ export abstract class Component<P, S> {
 	constructor(props?: P, context?: any);
 
 	static displayName?: string;
-	static defaultProps?: any;
 	static contextType?: Context<any>;
 
 	// Static members cannot reference class type parameters. This is not

diff --git a/test/browser/spec.test.js b/test/browser/spec.test.js
@@ -16,88 +16,6 @@ describe('Component spec', () => {
 		teardown(scratch);
 	});
 
-	describe('defaultProps', () => {
-		it('should apply default props on initial render', () => {
-			class WithDefaultProps extends Component {
-				constructor(props, context) {
-					super(props, context);
-					expect(props).to.be.deep.equal({
-						fieldA: 1,
-						fieldB: 2,
-						fieldC: 1,
-						fieldD: 2
-					});
-				}
-				render() {
-					return <div />;
-				}
-			}
-			WithDefaultProps.defaultProps = { fieldC: 1, fieldD: 1 };
-			render(<WithDefaultProps fieldA={1} fieldB={2} fieldD={2} />, scratch);
-		});
-
-		it('should apply default props on rerender', () => {
-			let doRender;
-			class Outer extends Component {
-				constructor() {
-					super();
-					this.state = { i: 1 };
-				}
-				componentDidMount() {
-					doRender = () => this.setState({ i: 2 });
-				}
-				render(props, { i }) {
-					return <WithDefaultProps fieldA={1} fieldB={i} fieldD={i} />;
-				}
-			}
-			class WithDefaultProps extends Component {
-				constructor(props, context) {
-					super(props, context);
-					this.ctor(props, context);
-				}
-				ctor() {}
-				componentWillReceiveProps() {}
-				render() {
-					return <div />;
-				}
-			}
-			WithDefaultProps.defaultProps = { fieldC: 1, fieldD: 1 };
-
-			let proto = WithDefaultProps.prototype;
-			sinon.spy(proto, 'ctor');
-			sinon.spy(proto, 'componentWillReceiveProps');
-			sinon.spy(proto, 'render');
-
-			render(<Outer />, scratch);
-			doRender();
-
-			const PROPS1 = {
-				fieldA: 1,
-				fieldB: 1,
-				fieldC: 1,
-				fieldD: 1
-			};
-
-			const PROPS2 = {
-				fieldA: 1,
-				fieldB: 2,
-				fieldC: 1,
-				fieldD: 2
-			};
-
-			expect(proto.ctor).to.have.been.calledWithMatch(PROPS1);
-			expect(proto.render).to.have.been.calledWithMatch(PROPS1);
-
-			rerender();
-
-			// expect(proto.ctor).to.have.been.calledWith(PROPS2);
-			expect(proto.componentWillReceiveProps).to.have.been.calledWithMatch(
-				PROPS2
-			);
-			expect(proto.render).to.have.been.calledWithMatch(PROPS2);
-		});
-	});
-
 	describe('forceUpdate', () => {
 		it('should force a rerender', () => {
 			let forceUpdate;

diff --git a/test/shared/createElement.test.js b/test/shared/createElement.test.js
@@ -270,12 +270,6 @@ describe('createElement(jsx)', () => {
 			.that.deep.equals(['x', 'y']);
 	});
 
-	it('should respect defaultProps', () => {
-		const Component = ({ children }) => children;
-		Component.defaultProps = { foo: 'bar' };
-		expect(h(Component).props).to.deep.equal({ foo: 'bar' });
-	});
-
 	it('should override defaultProps', () => {
 		const Component = ({ children }) => children;
 		Component.defaultProps = { foo: 'default' };