[GENERAL] [BUGFIX] [Image] Fix props width, height and tintColor#19281
[GENERAL] [BUGFIX] [Image] Fix props width, height and tintColor#19281brunolemos wants to merge 2 commits intofacebook:masterfrom
Conversation
…y over other props in this component
|
Ping @shergin @TheSavior this is a small one |
|
Hmm. We recommend putting those props in the If you set them in |
|
@TheSavior Thanks. Yes, style works as expected. I understand this is not the recommended way of doing it, but while RN still haven't moved away from it, I see no harm on including this fix, right? Like, all other props work, except for these three. |
|
Cool, glad We are actively removing this functionality, so I don't think we want to reverse direction on this and open the door to it being used more. |
@TheSavior can you point me to PRs or discussions about this? I wasn't able to find. |
|
I don't actually think we've posted about this anywhere yet (you're hearing it first!) :'( We have been working on rewriting the core of React Native (project is called Fabric) and want to be stricter with what props get passed in to native components. There are some perf wins to be gained here as well. One of the other problems we've had with styles being set as props are that if you define a prop on a component with the name of a style then you can run into conflicts. We have been flow typing the core components and disallowing unsupported props being passed in that way as a first step. To support open source better we need to get a dev mode runtime warning or something out so people can find and have time to update. Edit: Apparently there has been a TODO on these lines for the last 3 years. |
|
@TheSavior I'm removing the "styles as props" support from TypeScript definition (DefinitelyTyped/DefinitelyTyped#28217), please let me know if this is still the plan or if someone decided that it will keep working as today on Fabric. |
|
We still intend to remove styles as top level props and the React Native flow definitions enforce it as such. It would be good to have typescript be consistent with that. Thanks for making the change to the typescript definition! |
|
Ok! Thanks for the answer. |
|
TypeScript change merged and deployed. |
Test Plan
Currently this already works with the live version:
<View width={100} height={100} margin={100} /><Image width={100} height={100} source={[{ uri: '' }]} />// width and height when source is an array<Image margin={100} />// any style as prop, except tintColorThis only works after this pr:
<Image width={100} height={100} source={{ uri: '' } />// width and height when source is not an array<Image tintColor="#0000FF" />// tintColor as propRelease Notes
[GENERAL] [BUGFIX] [Image] Fix props width, height and tintColor