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

Remove unused injectComponentClasses and tagToComponentClass #8050

Merged
merged 2 commits into from
Oct 22, 2016
Merged

Remove unused injectComponentClasses and tagToComponentClass #8050

merged 2 commits into from
Oct 22, 2016

Conversation

diegomura
Copy link
Contributor

tagToComponentClass attribute of ReactHostComponent is not being used when creating an internal component, making useless to inject a Host Component class by the tag name.

The reconciler should check if there is a proper class implementation for a specific tag, and if not, return a generic component class instance.

@aweary
Copy link
Contributor

aweary commented Oct 22, 2016

Thanks for the PR @diegomura! It actually looks like we don't even use injectComponentClasses at all anymore. It's not used in react-dom, react-native or react-art.

You're change would be correct, but if we're not using it at all I wonder if we should just remove it? /cc @spicyj

@sophiebits
Copy link
Contributor

Yes, we should just delete this.

@aweary
Copy link
Contributor

aweary commented Oct 22, 2016

@diegomura do you want to update your PR to delete injectComponentClasses and tagToComponentClass instead? Good catch!

@diegomura
Copy link
Contributor Author

Thanks guys!

injectComponentClasses would actually be very handy for someone trying to create a React renderer (as I am 😁) because enables you to separate host component concerns in different files without writing a switch statement in the generic host component. But I guess letting users write renderers is outside React scope right now.
What do you think?

I have no problem on changing my PR if you think is for the best @spicyj @aweary

@aweary
Copy link
Contributor

aweary commented Oct 22, 2016

But I guess letting users write renderers is outside React scope right now.

Yeah, I don't think there are any plans for supporting a public API for custom renderers with this reconciler at the moment. We should treat this code as internal.

@sophiebits
Copy link
Contributor

Yeah, putting the switch/lookup/delegation code in your own host component would be best.

@diegomura
Copy link
Contributor Author

It's done! Thanks @aweary @spicyj !!

@aweary aweary changed the title Injected Host Component classes are not being considered by the reconciler Remove unused injectComponentClasses and tagToComponentClass Oct 22, 2016
@aweary aweary added this to the 15-next milestone Oct 22, 2016
@aweary aweary merged commit 461a741 into facebook:master Oct 22, 2016
@aweary
Copy link
Contributor

aweary commented Oct 22, 2016

Thank you @diegomura!

@diegomura diegomura deleted the instantiate-host-components branch November 2, 2016 02:50
@gaearon gaearon modified the milestones: 15-hipri, 15-lopri, 15.4.2 Jan 6, 2017
gaearon pushed a commit that referenced this pull request Jan 6, 2017
…ciler (#8050)

* Consider Host Component classes when creating a new internal instance

* Remove unused tagToComponentClass & injectComponentClasses from ReactHostComponent

(cherry picked from commit 461a741)
acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
…ciler (facebook#8050)

* Consider Host Component classes when creating a new internal instance

* Remove unused tagToComponentClass & injectComponentClasses from ReactHostComponent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants