Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated misleading error message in production environment when adding ref to a functional component (#11761) #11782

Merged
merged 4 commits into from
Jan 5, 2018
Merged

Updated misleading error message in production environment when adding ref to a functional component (#11761) #11782

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c280f65
Merge pull request #1 from facebook/master
skiritsis Nov 26, 2017
a48dd35
Updated misleading error message in production environment when addin…
skiritsis Dec 6, 2017
4b62acd
Reverted changes to codes.json
skiritsis Dec 6, 2017
99f9939
Updated error message
skiritsis Dec 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions packages/react-dom/src/__tests__/ReactStatelessComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,11 @@ describe('ReactStatelessComponent', () => {
}).toThrowError(
__DEV__
? 'Stateless function components cannot have refs.'
: // TODO: the different message in production seems like a bug.
// It happens because we don't save _owner in production for
// functional components. We should probably show a better message.
'Element ref was specified as a string (me) but no owner was set.',
: // It happens because we don't save _owner in production for
// functional components.
'Element ref was specified as a string (me) but no owner was set. ' +
"You may be adding a ref to a component that was not created inside a component's render method, or " +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this message doesn't make it clear that this could be due to using a string ref in a functional component.
Can we split it in three parts? e.g.

Element ref was specified as a string (me) but no owner was set. This could happen for one of the following reasons:

1.
2.
3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course. I will update it accordingly.

'you have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).',
);
});

Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/__tests__/multiple-copies-of-react-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ describe('when different React version is used with string ref', () => {
expect(() => {
ReactTestUtils.renderIntoDocument(<TextWithStringRef />);
}).toThrow(
'Element ref was specified as a string (foo) but no owner was set.' +
' You may have multiple copies of React loaded. (details: ' +
'https://fb.me/react-refs-must-have-owner).',
'Element ref was specified as a string (foo) but no owner was set. ' +
"You may be adding a ref to a component that was not created inside a component's render method, or " +
'you have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).',
);
});
});
6 changes: 3 additions & 3 deletions packages/react-dom/src/__tests__/refs-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,9 @@ describe('creating element with ref in constructor', () => {
expect(function() {
ReactTestUtils.renderIntoDocument(<RefTest />);
}).toThrowError(
'Element ref was specified as a string (p) but no owner was ' +
'set. You may have multiple copies of React loaded. ' +
'(details: https://fb.me/react-refs-must-have-owner).',
'Element ref was specified as a string (p) but no owner was set. ' +
"You may be adding a ref to a component that was not created inside a component's render method, or " +
'you have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).',
);
});
});
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactChildFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ function coerceRef(current: Fiber | null, element: ReactElement) {
);
invariant(
element._owner,
'Element ref was specified as a string (%s) but no owner was ' +
'set. You may have multiple copies of React loaded. ' +
'(details: https://fb.me/react-refs-must-have-owner).',
'Element ref was specified as a string (%s) but no owner was set. ' +
"You may be adding a ref to a component that was not created inside a component's render method, or " +
'you have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).',
mixedRef,
);
}
Expand Down
2 changes: 1 addition & 1 deletion scripts/error-codes/codes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
"146": "React Component in warnForMissingKey should have a _store. This error is likely caused by a bug in React. Please file an issue.",
"147": "Missing owner for string ref %s. This error is likely caused by a bug in React. Please file an issue.",
"148": "Expected ref to be a function or a string.",
"149": "Element ref was specified as a string (%s) but no owner was set. You may have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).",
"149": "Element ref was specified as a string (%s) but no owner was set. You may be adding a ref to a component that was not created inside a component's render method, or you have multiple copies of React loaded. (details: https://fb.me/react-refs-must-have-owner).",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't manually edit this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, was totally unaware of that. Reverted the changes in codes.json

"150": "An object is not an iterable. This error is likely caused by a bug in React. Please file an issue.",
"151": "An iterable object provided no iterator.",
"152": "%s(...): Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.",
Expand Down