@samitouri / QOS-React / commits / d1542de3a6

Unify React.memo and React.forwardRef display name logic (#21392)

Co-authored-by: iChenLei <2470828450@qq.com>

Brian Vaughn committed May 4, 2021 at 11:40 UTC d1542de3a607024421e13db01a42d8ba5930c389

8 files changed +220 -42

packages/react-devtools-shared/src/__tests__/__snapshots__/store-test.js.snap

-17

@@ -670,20 +670,3 @@ exports[`Store should properly serialize non-string key values: 1: mount 1`] = `

670 [root]

671 <Child key="123">

672 `;

673 -

674 -exports[`Store should show the right display names for special component types 1`] = `

675 -[root]

676 - ▾ <App>

677 - <MyComponent>

678 - <MyComponent> [ForwardRef]

679 - ▾ <Anonymous> [ForwardRef]

680 - <MyComponent2>

681 - <Custom> [ForwardRef]

682 - <MyComponent4> [Memo]

683 - ▾ <MyComponent> [Memo]

684 - <MyComponent> [ForwardRef]

685 - <Baz> [withFoo][withBar]

686 - <Baz> [Memo][withFoo][withBar]

687 - <Baz> [ForwardRef][withFoo][withBar]

688 - <Cache>

689 -`;

packages/react-devtools-shared/src/__tests__/store-test.js

+31 -1

@@ -879,6 +879,17 @@ describe('Store', () => {

879 FakeHigherOrderComponent,

880 );

881

882 + const MemoizedFakeHigherOrderComponentWithDisplayNameOverride = React.memo(

883 + FakeHigherOrderComponent,

884 + );

885 + MemoizedFakeHigherOrderComponentWithDisplayNameOverride.displayName =

886 + 'memoRefOverride';

887 + const ForwardRefFakeHigherOrderComponentWithDisplayNameOverride = React.forwardRef(

888 + FakeHigherOrderComponent,

889 + );

890 + ForwardRefFakeHigherOrderComponentWithDisplayNameOverride.displayName =

891 + 'forwardRefOverride';

892 +

893 const App = () => (

894 <React.Fragment>

895 <MyComponent />

@@ -891,6 +902,8 @@ describe('Store', () => {

902 <MemoizedFakeHigherOrderComponent />

903 <ForwardRefFakeHigherOrderComponent />

904 <React.unstable_Cache />

905 + <MemoizedFakeHigherOrderComponentWithDisplayNameOverride />

906 + <ForwardRefFakeHigherOrderComponentWithDisplayNameOverride />

907 </React.Fragment>

908 );

909

@@ -904,7 +917,24 @@ describe('Store', () => {

917 // Render again after it resolves

918 act(() => ReactDOM.render(<App />, container));

919

907 - expect(store).toMatchSnapshot();

920 + expect(store).toMatchInlineSnapshot(`

921 + [root]

922 + ▾ <App>

923 + <MyComponent>

924 + <MyComponent> [ForwardRef]

925 + ▾ <Anonymous> [ForwardRef]

926 + <MyComponent2>

927 + <Custom> [ForwardRef]

928 + <MyComponent4> [Memo]

929 + ▾ <MyComponent> [Memo]

930 + <MyComponent> [ForwardRef]

931 + <Baz> [withFoo][withBar]

932 + <Baz> [Memo][withFoo][withBar]

933 + <Baz> [ForwardRef][withFoo][withBar]

934 + <Cache>

935 + <memoRefOverride> [Memo]

936 + <forwardRefOverride> [ForwardRef]

937 + `);

938 });

939

940 describe('Lazy', () => {

packages/react-devtools-shared/src/backend/renderer.js

+6 -2

@@ -393,7 +393,7 @@ export function getInternalReactConstants(

393

394 // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods

395 function getDisplayNameForFiber(fiber: Fiber): string | null {

396 - const {type, tag} = fiber;

396 + const {elementType, type, tag} = fiber;

397

398 let resolvedType = type;

399 if (typeof type === 'object' && type !== null) {

@@ -432,7 +432,11 @@ export function getInternalReactConstants(

432 return 'Lazy';

433 case MemoComponent:

434 case SimpleMemoComponent:

435 - return getDisplayName(resolvedType, 'Anonymous');

435 + return (

436 + (elementType && elementType.displayName) ||

437 + (type && type.displayName) ||

438 + getDisplayName(resolvedType, 'Anonymous')

439 + );

440 case SuspenseComponent:

441 return 'Suspense';

442 case LegacyHiddenComponent:

packages/react-reconciler/src/__tests__/ReactMemo-test.js

+63 -8

@@ -498,11 +498,66 @@ describe('memo', () => {

498 });

499 });

500

501 + it('should fall back to showing something meaningful if no displayName or name are present', () => {

502 + const MemoComponent = React.memo(props => <div {...props} />);

503 + MemoComponent.propTypes = {

504 + required: PropTypes.string.isRequired,

505 + };

506 +

507 + expect(() =>

508 + ReactNoop.render(<MemoComponent optional="foo" />),

509 + ).toErrorDev(

510 + 'Warning: Failed prop type: The prop `required` is marked as required in ' +

511 + '`Memo`, but its value is `undefined`.',

512 + // There's no component stack in this warning because the inner function is anonymous.

513 + // If we wanted to support this (for the Error frames / source location)

514 + // we could do this by updating ReactComponentStackFrame.

515 + {withoutStack: true},

516 + );

517 + });

518 +

519 + it('should honor a displayName if set on the inner component in warnings', () => {

520 + function Component(props) {

521 + return <div {...props} />;

522 + }

523 + Component.displayName = 'Inner';

524 + const MemoComponent = React.memo(Component);

525 + MemoComponent.propTypes = {

526 + required: PropTypes.string.isRequired,

527 + };

528 +

529 + expect(() =>

530 + ReactNoop.render(<MemoComponent optional="foo" />),

531 + ).toErrorDev(

532 + 'Warning: Failed prop type: The prop `required` is marked as required in ' +

533 + '`Inner`, but its value is `undefined`.\n' +

534 + ' in Inner (at **)',

535 + );

536 + });

537 +

538 it('should honor a displayName if set on the memo wrapper in warnings', () => {

539 const MemoComponent = React.memo(function Component(props) {

540 return <div {...props} />;

541 });

505 - MemoComponent.displayName = 'Foo';

542 + MemoComponent.displayName = 'Outer';

543 + MemoComponent.propTypes = {

544 + required: PropTypes.string.isRequired,

545 + };

546 +

547 + expect(() =>

548 + ReactNoop.render(<MemoComponent optional="foo" />),

549 + ).toErrorDev(

550 + 'Warning: Failed prop type: The prop `required` is marked as required in ' +

551 + '`Outer`, but its value is `undefined`.\n' +

552 + ' in Component (at **)',

553 + );

554 + });

555 +

556 + it('should pass displayName to an anonymous inner component so it shows up in component stacks', () => {

557 + const MemoComponent = React.memo(props => {

558 + return <div {...props} />;

559 + });

560 + MemoComponent.displayName = 'Memo';

561 MemoComponent.propTypes = {

562 required: PropTypes.string.isRequired,

563 };

@@ -511,19 +566,19 @@ describe('memo', () => {

566 ReactNoop.render(<MemoComponent optional="foo" />),

567 ).toErrorDev(

568 'Warning: Failed prop type: The prop `required` is marked as required in ' +

514 - '`Foo`, but its value is `undefined`.\n' +

515 - ' in Foo (at **)',

569 + '`Memo`, but its value is `undefined`.\n' +

570 + ' in Memo (at **)',

571 );

572 });

573

519 - it('should honor a inner displayName if set on the wrapped function', () => {

574 + it('should honor a outer displayName when wrapped component and memo component set displayName at the same time.', () => {

575 function Component(props) {

576 return <div {...props} />;

577 }

523 - Component.displayName = 'Foo';

578 + Component.displayName = 'Inner';

579

580 const MemoComponent = React.memo(Component);

526 - MemoComponent.displayName = 'Bar';

581 + MemoComponent.displayName = 'Outer';

582 MemoComponent.propTypes = {

583 required: PropTypes.string.isRequired,

584 };

@@ -532,8 +587,8 @@ describe('memo', () => {

587 ReactNoop.render(<MemoComponent optional="foo" />),

588 ).toErrorDev(

589 'Warning: Failed prop type: The prop `required` is marked as required in ' +

535 - '`Foo`, but its value is `undefined`.\n' +

536 - ' in Foo (at **)',

590 + '`Outer`, but its value is `undefined`.\n' +

591 + ' in Inner (at **)',

592 );

593 });

594 }

packages/react/src/ReactForwardRef.js

+9 -1

@@ -57,7 +57,15 @@ export function forwardRef<Props, ElementType: React$ElementType>(

57 },

58 set: function(name) {

59 ownName = name;

60 - if (render.displayName == null) {

60 +

61 + // The inner component shouldn't inherit this display name in most cases,

62 + // because the component may be used elsewhere.

63 + // But it's nice for anonymous functions to inherit the name,

64 + // so that our component-stack generation logic will display their frames.

65 + // An anonymous function generally suggests a pattern like:

66 + // React.forwardRef((props, ref) => {...});

67 + // This kind of inner function is not used elsewhere so the side effect is okay.

68 + if (!render.name && !render.displayName) {

69 render.displayName = name;

70 }

71 },

packages/react/src/ReactMemo.js

+9 -1

@@ -37,7 +37,15 @@ export function memo<Props>(

37 },

38 set: function(name) {

39 ownName = name;

40 - if (type.displayName == null) {

40 +

41 + // The inner component shouldn't inherit this display name in most cases,

42 + // because the component may be used elsewhere.

43 + // But it's nice for anonymous functions to inherit the name,

44 + // so that our component-stack generation logic will display their frames.

45 + // An anonymous function generally suggests a pattern like:

46 + // React.memo((props) => {...});

47 + // This kind of inner function is not used elsewhere so the side effect is okay.

48 + if (!type.name && !type.displayName) {

49 type.displayName = name;

50 }

51 },

packages/react/src/__tests__/forwardRef-test.js

+92 -7

@@ -217,14 +217,43 @@ describe('forwardRef', () => {

217 );

218 });

219

220 - it('should honor a displayName if set on the forwardRef wrapper in warnings', () => {

220 + it('should fall back to showing something meaningful if no displayName or name are present', () => {

221 const Component = props => <div {...props} />;

222

223 const RefForwardingComponent = React.forwardRef((props, ref) => (

224 <Component {...props} forwardedRef={ref} />

225 ));

226

227 - RefForwardingComponent.displayName = 'Foo';

227 + RefForwardingComponent.propTypes = {

228 + optional: PropTypes.string,

229 + required: PropTypes.string.isRequired,

230 + };

231 +

232 + RefForwardingComponent.defaultProps = {

233 + optional: 'default',

234 + };

235 +

236 + const ref = React.createRef();

237 +

238 + expect(() =>

239 + ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />),

240 + ).toErrorDev(

241 + 'Warning: Failed prop type: The prop `required` is marked as required in ' +

242 + '`ForwardRef`, but its value is `undefined`.',

243 + // There's no component stack in this warning because the inner function is anonymous.

244 + // If we wanted to support this (for the Error frames / source location)

245 + // we could do this by updating ReactComponentStackFrame.

246 + {withoutStack: true},

247 + );

248 + });

249 +

250 + it('should honor a displayName if set on the forwardRef wrapper in warnings', () => {

251 + const Component = props => <div {...props} />;

252 +

253 + const RefForwardingComponent = React.forwardRef(function Inner(props, ref) {

254 + <Component {...props} forwardedRef={ref} />;

255 + });

256 + RefForwardingComponent.displayName = 'Custom';

257

258 RefForwardingComponent.propTypes = {

259 optional: PropTypes.string,

@@ -241,8 +270,36 @@ describe('forwardRef', () => {

270 ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />),

271 ).toErrorDev(

272 'Warning: Failed prop type: The prop `required` is marked as required in ' +

244 - '`Foo`, but its value is `undefined`.\n' +

245 - ' in Foo (at **)',

273 + '`Custom`, but its value is `undefined`.\n' +

274 + ' in Inner (at **)',

275 + );

276 + });

277 +

278 + it('should pass displayName to an anonymous inner component so it shows up in component stacks', () => {

279 + const Component = props => <div {...props} />;

280 +

281 + const RefForwardingComponent = React.forwardRef((props, ref) => (

282 + <Component {...props} forwardedRef={ref} />

283 + ));

284 + RefForwardingComponent.displayName = 'Custom';

285 +

286 + RefForwardingComponent.propTypes = {

287 + optional: PropTypes.string,

288 + required: PropTypes.string.isRequired,

289 + };

290 +

291 + RefForwardingComponent.defaultProps = {

292 + optional: 'default',

293 + };

294 +

295 + const ref = React.createRef();

296 +

297 + expect(() =>

298 + ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />),

299 + ).toErrorDev(

300 + 'Warning: Failed prop type: The prop `required` is marked as required in ' +

301 + '`Custom`, but its value is `undefined`.\n' +

302 + ' in Custom (at **)',

303 );

304 });

305

@@ -250,8 +307,36 @@ describe('forwardRef', () => {

307 const Component = props => <div {...props} />;

308

309 const inner = (props, ref) => <Component {...props} forwardedRef={ref} />;

253 - inner.displayName = 'Foo';

310 + inner.displayName = 'Inner';

311 + const RefForwardingComponent = React.forwardRef(inner);

312 +

313 + RefForwardingComponent.propTypes = {

314 + optional: PropTypes.string,

315 + required: PropTypes.string.isRequired,

316 + };

317 +

318 + RefForwardingComponent.defaultProps = {

319 + optional: 'default',

320 + };

321 +

322 + const ref = React.createRef();

323 +

324 + expect(() =>

325 + ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />),

326 + ).toErrorDev(

327 + 'Warning: Failed prop type: The prop `required` is marked as required in ' +

328 + '`ForwardRef(Inner)`, but its value is `undefined`.\n' +

329 + ' in Inner (at **)',

330 + );

331 + });

332 +

333 + it('should honor a outer displayName when wrapped component and memo component set displayName at the same time.', () => {

334 + const Component = props => <div {...props} />;

335 +

336 + const inner = (props, ref) => <Component {...props} forwardedRef={ref} />;

337 + inner.displayName = 'Inner';

338 const RefForwardingComponent = React.forwardRef(inner);

339 + RefForwardingComponent.displayName = 'Outer';

340

341 RefForwardingComponent.propTypes = {

342 optional: PropTypes.string,

@@ -268,8 +353,8 @@ describe('forwardRef', () => {

353 ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />),

354 ).toErrorDev(

355 'Warning: Failed prop type: The prop `required` is marked as required in ' +

271 - '`ForwardRef(Foo)`, but its value is `undefined`.\n' +

272 - ' in Foo (at **)',

356 + '`Outer`, but its value is `undefined`.\n' +

357 + ' in Inner (at **)',

358 );

359 });

360

packages/shared/getComponentNameFromType.js

+10 -5

@@ -31,11 +31,12 @@ function getWrappedName(

31 innerType: any,

32 wrapperName: string,

33 ): string {

34 + const displayName = (outerType: any).displayName;

35 + if (displayName) {

36 + return displayName;

37 + }

38 const functionName = innerType.displayName || innerType.name || '';

35 - return (

36 - (outerType: any).displayName ||

37 - (functionName !== '' ? `${wrapperName}(${functionName})` : wrapperName)

38 - );

39 + return functionName !== '' ? `${wrapperName}(${functionName})` : wrapperName;

40 }

42 // Keep in sync with react-reconciler/getComponentNameFromFiber

@@ -90,7 +91,11 @@ export default function getComponentNameFromType(type: mixed): string | null {

91 case REACT_FORWARD_REF_TYPE:

92 return getWrappedName(type, type.render, 'ForwardRef');

93 case REACT_MEMO_TYPE:

93 - return getComponentNameFromType(type.type);

94 + const outerName = (type: any).displayName || null;

95 + if (outerName !== null) {

96 + return outerName;

97 + }

98 + return getComponentNameFromType(type.type) || 'Memo';

99 case REACT_LAZY_TYPE: {

100 const lazyComponent: LazyComponent<any, any> = (type: any);

101 const payload = lazyComponent._payload;