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.

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

No error when element with ref created in another elements constructor in dev environment. #9635

Closed
monkeyboy64 opened this issue May 9, 2017 · 8 comments

Comments

@monkeyboy64
Copy link

When creating an Element with a ref from inside of a constructor an error is thrown in production mode, but not in dev mode.

Here is a minimal sample:
https://jsfiddle.net/84v837e9/35/

An error, "Only a ReactOwner can have refs", should happen in dev and production modes.

This is happening in React 15.4

@LemonPi
Copy link

LemonPi commented Jun 4, 2017

I also encountered the same issue, originally posted as issue facebook/create-react-app#2460 in the CRA repo.

Description

According to the docs refs should only be assigned inside a component's render() method. I didn't know this and created a composed component inside the constructor, assigning its ref there.

Testing with npm start runs without any errors or warnings, but running the production build after
npm run build
serve -s build
Produced an error and the rest of the application stopped working.

Expected behavior

npm start should also give the same error. I expect if the development application works then the production one will as well.

Actual behavior

No error or warning under npm start.

Environment

Run these commands in the project folder and fill in their results:

  1. npm ls react-scripts (if you haven’t ejected): react-scripts@1.0.2
  2. node -v: v6.9.4
  3. npm -v: 3.10.10
  4. react 15.5.4

Then, specify:

  1. Operating system: Windows 10 Home Version 1607
  2. Browser and version: Google Chrome 58.0.3029.110 (64-bit)

Reproducible Demo

See project: https://github.com/LemonPi/cra-refs-bug with a readme on what it demonstrates
See deployed version that errors out: https://lemonpi.github.io/cra-refs-bug/ or npm run build and serve -s build

@gaearon
Copy link
Collaborator

gaearon commented Jun 22, 2017

This likely is a bug. I have a hunch it might be already fixed on master. I'd appreciate if somebody could confirm that by writing a failing test.

@iansu
Copy link
Contributor

iansu commented Jun 22, 2017

@gaearon I've been following this thread and would be interested in verifying the bug and attempting to fix it if it does still exist. Unless either of the bug reporters would like to tackle it.

@gaearon
Copy link
Collaborator

gaearon commented Jun 22, 2017

Sure, go ahead! You'll need to look for assignments to ReactCurrentOwner.current which is what seems to deviate in production. I think deviation was intentional but we either didn't consider this case (breaking refs in constructor) or we had some protection against it that got lost at some point.

@iansu
Copy link
Contributor

iansu commented Jun 22, 2017

@gaearon I've added a test to verify that this bug does still exist on master. I'll look into fixing the bug next.

@aweary aweary mentioned this issue Jun 26, 2017
12 tasks
@nhunzaker nhunzaker mentioned this issue Aug 10, 2017
11 tasks
@aweary
Copy link
Contributor

aweary commented Oct 4, 2017

This was fixed with #10025

@aweary aweary closed this as completed Oct 4, 2017
@iansu
Copy link
Contributor

iansu commented Oct 4, 2017

@aweary I think you mean it was fixed with #10025. 🙂

@aweary
Copy link
Contributor

aweary commented Oct 4, 2017

@iansu thanks, copy-paste error 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants