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

Warn for string refs where owner != __self #7091

Closed
jimfb opened this issue Jun 20, 2016 · 15 comments
Closed

Warn for string refs where owner != __self #7091

jimfb opened this issue Jun 20, 2016 · 15 comments
Assignees

Comments

@jimfb
Copy link
Contributor

jimfb commented Jun 20, 2016

Sebastian wants to warn when owner !== __self, because this is the hard case to find when codemodding from string refs to callback refs.

@aweary
Copy link
Contributor

aweary commented Jun 20, 2016

Would this likely be done in ReactOwner.addComponentAsRefTo?

@jimfb
Copy link
Contributor Author

jimfb commented Jun 20, 2016

Probably would be a separate dev tool that sanity checked the element when the element is being mounted. But we'll need to fix the transform first, since I don't think we're even using the __self prop right now, IIRC.

@aweary
Copy link
Contributor

aweary commented Jun 20, 2016

Which transform are you referring to? I'd like to take a look at this if possible.

I don't think we're even using the __source prop

Do you mean __self or is __source required here too?

@jimfb
Copy link
Contributor Author

jimfb commented Jun 20, 2016

Sorry, yeah, __self, edited/fixed in reply above.

Which transform are you referring to? I'd like to take a look at this if possible.

babel/babel#3540

@keyz
Copy link
Contributor

keyz commented Jun 21, 2016

How would this affect people who use createElement explicitly?

@jimfb
Copy link
Contributor Author

jimfb commented Jun 21, 2016

@keyanzhang They miss out on a potentially useful warning. Unfortunately, there really isn't much we can do to make their experience any better, AFAIK.

@jimfb
Copy link
Contributor Author

jimfb commented Jun 21, 2016

cc @facebook/react-core Public API change.

@sophiebits
Copy link
Collaborator

Well you could also make a transform that adds __self to React.createElement calls.

We always wanted to log this internally – not 100% sure it makes sense to give a warning externally too? I guess it's probably reasonable.

@stale
Copy link

stale bot commented Jan 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@sebmarkbage
Copy link
Collaborator

This is something we actually should do soon as part of deprecations work. If we first get rid of these edge cases then it will allow us to write a codemod for string refs.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Apr 9, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2020
@gaearon
Copy link
Collaborator

gaearon commented Apr 9, 2020

bump

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2020
@stale
Copy link

stale bot commented Jul 11, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jul 11, 2020
@rickhanlonii
Copy link
Member

Bump

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jul 13, 2020
@lunaruan
Copy link
Contributor

This is closed by #17864.

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

8 participants