diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index e8d3b4ae755..8370d371050 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1538,6 +1538,7 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js * should throw on string refs in pure functions * should warn when given a string ref * should warn when given a function ref +* deduplicates ref warnings based on element or owner * should provide a null ref * should use correct name in key warning * should support default props and prop types diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index db863f1783a..9401be93d13 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -67,6 +67,8 @@ var warning = require('warning'); if (__DEV__) { var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); + + var warnedAboutStatelessRefs = {}; } module.exports = function( @@ -464,13 +466,21 @@ module.exports = function( info += ' Check the render method of `' + ownerName + '`.'; } - warning( - false, - 'Stateless function components cannot be given refs. ' + - 'Attempts to access this ref will fail.%s%s', - info, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() - ); + let warningKey = ownerName || workInProgress._debugID || ''; + const debugSource = workInProgress._debugSource; + if (debugSource) { + warningKey = debugSource.fileName + ':' + debugSource.lineNumber; + } + if (!warnedAboutStatelessRefs[warningKey]) { + warnedAboutStatelessRefs[warningKey] = true; + warning( + false, + 'Stateless function components cannot be given refs. ' + + 'Attempts to access this ref will fail.%s%s', + info, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() + ); + } } } reconcileChildren(current, workInProgress, value); diff --git a/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js b/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js index ec63bbd70b4..060ec461373 100644 --- a/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js +++ b/src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js @@ -25,6 +25,7 @@ describe('ReactStatelessComponent', () => { } beforeEach(() => { + jest.resetModuleRegistry(); React = require('React'); ReactDOM = require('ReactDOM'); ReactTestUtils = require('ReactTestUtils'); @@ -156,54 +157,131 @@ describe('ReactStatelessComponent', () => { return
{props.children}
; } - class Parent extends React.Component { + class ParentUsingStringRef extends React.Component { render() { - return ; + return ( + + + + ); } } - ReactTestUtils.renderIntoDocument(); - + ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(1); expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( 'Warning: Stateless function components cannot be given refs. ' + 'Attempts to access this ref will fail. Check the render method ' + - 'of `Parent`.\n' + + 'of `ParentUsingStringRef`.\n' + ' in StatelessComponent (at **)\n' + ' in div (at **)\n' + ' in Indirection (at **)\n' + - ' in Parent (at **)' + ' in ParentUsingStringRef (at **)' ); + + ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); }); it('should warn when given a function ref', () => { spyOn(console, 'error'); - var ref = jasmine.createSpy().and.callFake((arg) => { - expect(arg).toBe(null); - }); function Indirection(props) { return
{props.children}
; } - class Parent extends React.Component { + class ParentUsingFunctionRef extends React.Component { render() { - return ; + return ( + + { + expect(arg).toBe(null); + }} /> + + ); } } - ReactTestUtils.renderIntoDocument(); - + ReactTestUtils.renderIntoDocument(); expectDev(console.error.calls.count()).toBe(1); expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( 'Warning: Stateless function components cannot be given refs. ' + 'Attempts to access this ref will fail. Check the render method ' + - 'of `Parent`.\n' + + 'of `ParentUsingFunctionRef`.\n' + ' in StatelessComponent (at **)\n' + ' in div (at **)\n' + ' in Indirection (at **)\n' + - ' in Parent (at **)' + ' in ParentUsingFunctionRef (at **)' + ); + + ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); + }); + + it('deduplicates ref warnings based on element or owner', () => { + spyOn(console, 'error'); + + // Prevent the Babel transform adding a displayName. + var createClassWithoutDisplayName = React.createClass; + + // When owner uses JSX, we can use exact line location to dedupe warnings + var AnonymousParentUsingJSX = createClassWithoutDisplayName({ + render() { + return {}} />; + }, + }); + const instance1 = ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Stateless function components cannot be given refs.' ); + // Should be deduped (offending element is on the same line): + instance1.forceUpdate(); + // Should also be deduped (offending element is on the same line): + ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); + console.error.calls.reset(); + + // When owner doesn't use JSX, and is anonymous, we warn once per internal instance. + var AnonymousParentNotUsingJSX = createClassWithoutDisplayName({ + render() { + return React.createElement(StatelessComponent, {name: 'A', 'ref': () => {}}); + }, + }); + const instance2 = ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Stateless function components cannot be given refs.' + ); + // Should be deduped (same internal instance): + instance2.forceUpdate(); + expectDev(console.error.calls.count()).toBe(1); + // Could not be deduped (different internal instance): + ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(2); + expectDev(console.error.calls.argsFor(1)[0]).toContain( + 'Warning: Stateless function components cannot be given refs.' + ); + console.error.calls.reset(); + + // When owner doesn't use JSX, but is named, we warn once per owner name + class NamedParentNotUsingJSX extends React.Component { + render() { + return React.createElement(StatelessComponent, {name: 'A', 'ref': () => {}}); + } + } + const instance3 = ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); + expectDev(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: Stateless function components cannot be given refs.' + ); + // Should be deduped (same owner name): + instance3.forceUpdate(); + expectDev(console.error.calls.count()).toBe(1); + // Should also be deduped (same owner name): + ReactTestUtils.renderIntoDocument(); + expectDev(console.error.calls.count()).toBe(1); + console.error.calls.reset(); }); it('should provide a null ref', () => { diff --git a/src/renderers/shared/stack/reconciler/ReactRef.js b/src/renderers/shared/stack/reconciler/ReactRef.js index 762e29685cd..6e1d3681b9b 100644 --- a/src/renderers/shared/stack/reconciler/ReactRef.js +++ b/src/renderers/shared/stack/reconciler/ReactRef.js @@ -23,28 +23,40 @@ if (__DEV__) { var ReactCompositeComponentTypes = require('ReactCompositeComponentTypes'); var ReactComponentTreeHook = require('ReactComponentTreeHook'); var warning = require('warning'); + + var warnedAboutStatelessRefs = {}; } function attachRef(ref, component, owner) { if (__DEV__) { - let info = ''; - if (owner) { + if (component._compositeType === ReactCompositeComponentTypes.StatelessFunctional) { + let info = ''; let ownerName; - if (typeof owner.getName === 'function') { - ownerName = owner.getName(); + if (owner) { + if (typeof owner.getName === 'function') { + ownerName = owner.getName(); + } + if (ownerName) { + info += ' Check the render method of `' + ownerName + '`.'; + } + } + + let warningKey = ownerName || component._debugID; + let element = component._currentElement; + if (element && element._source) { + warningKey = element._source.fileName + ':' + element._source.lineNumber; } - if (ownerName) { - info += ' Check the render method of `' + ownerName + '`.'; + if (!warnedAboutStatelessRefs[warningKey]) { + warnedAboutStatelessRefs[warningKey] = true; + warning( + false, + 'Stateless function components cannot be given refs. ' + + 'Attempts to access this ref will fail.%s%s', + info, + ReactComponentTreeHook.getStackAddendumByID(component._debugID) + ); } } - - warning( - component._compositeType !== ReactCompositeComponentTypes.StatelessFunctional, - 'Stateless function components cannot be given refs. ' + - 'Attempts to access this ref will fail.%s%s', - info, - ReactComponentTreeHook.getStackAddendumByID(component._debugID) - ); } if (typeof ref === 'function') {