godaddy / svgs

svgs is a compatiblity layer between svg and react-native-svg
MIT License
191 stars 18 forks source link

svgs component cannot receive the style created by StyleSheet in web #39

Open sasurau4 opened 5 years ago

sasurau4 commented 5 years ago

Overview

svgs's component can't receive style created by StyleSheet.create() in web.

Details

I'm developing an application with react-native and react-native-web. react-native-web is porting react-native's implementation as much as possible. So, StyleSheet.create() return object style key and id like following.

import { StyleSheet } from "react-native";

const styles = StyleSheet.create({
  container: {
    flex: 1
  }
});

console.log(styles.container) // -> 73 (style id)

react-native-web resolve all styles in it's components. https://github.com/necolas/react-native-web/blob/9872c9716973870ab3cc129215a17fc6cbb1b112/packages/react-native-web/src/exports/StyleSheet/ReactNativeStyleResolver.js

Actual Behavior

Throws Error of following and nothing to render.

Uncaught Invariant Violation: The `style` prop expects a mapping from style properties to values, not a string. For example, style={{marginRight: spacing + 'em'}} when using JSX.

Expected Behavior

Correctly svg rendered.

Reproduction repository

https://github.com/sasurau4/sample-rnweb/tree/repro/svgs-issue


I think this is edge case. But if this issue fixed, svgs fully compatible to the react-native-svg.

Thanks in advance.

3rd-Eden commented 5 years ago

Thanks for taking the time to report and thoroughly describe the issue.

So just to be clear here, the issue is compatibility with React-Native-Web, as it using the React-Native pattern for styles for web, instead of returning actual object as expected by React it returns numbers.

We don't use React-Native-Web at all, so this is low priority issue for us. It's an issue nonetheless. What I do I provide you with a decent work around until a PR is made, or contributed. As stated in the proposals and discussions repository of React-Native:

Many of us think that StyleSheet API does optimize styles. But it never did and since 0.55 it's an identity function. It's not really clear to me how to write future proof styles today as I don't know the internal FB discussions about this API.

There is absolutely no benefit of using the Stylesheet API vs passing regular objects. So I would suggest stop using the StyleSheet API, use regular objects, and everything will work on React-Native as well as normal React (even without React-Native-Web).

sasurau4 commented 5 years ago

Thanks for your response and sharing the proposals and discussions issue.👍

I read the issue, FB commented:

Comment by Facebook: This issue assumes that styles with React Native are not performant, which is not the case so there is nothing to be done to optimize them at this time. There may be an opportunity for us to improve our documentation to make sure people do not assume that the StyleSheet API is slow. We will not be adding a new API for defining styles at this time but that shouldn't stop you from exploring alternative solutions and building your own React Native styling libraries.

According from the comment, it has benefit for using StyleSheet API even though the performance section was removed from the StyleSheet docs. I don't know why the section was removed 🤔 and the future of StyleSheet API. So, I continue watching react-native repo and its community.

I understood the stance to the issue and priority for you. I'll look for a workaround or use passing regular object partially about svgs components instead of StyleSheet API.

If the issue is bothered you, please close the issue.