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

React native support #596

Closed
zRelux opened this issue May 7, 2021 · 39 comments
Closed

React native support #596

zRelux opened this issue May 7, 2021 · 39 comments
Labels
awaiting feedback Awaiting feedback

Comments

@zRelux
Copy link

zRelux commented May 7, 2021

I really love what you guys have been doing with stitches and have been loving it so far!

Any idea if react native is in the roadmap, I think the community would benefit a lot from this

@Temzasse
Copy link
Contributor

Hi 👋🏻

I've been working on the RN version of Stitches here: https://github.com/Temzasse/stitches-native

The core functionality is pretty much ready but I'm quite badly stuck with the TS typings. Any help with the typings would be very much welcome! 👐🏻

@peduarte do you have any tips about how could I utilize the existing types of Stitches in the RN version? I see that you have gone the declaration route instead of adding types to the implementation itself. I've been trying to type the implementation in my version and it gets super tricky 😄 Would you be open to sharing the learnings of your TS journey with Stitches?

@peduarte
Copy link
Contributor

@Temzasse hi! Nice 💯

utilize the existing types of Stitches in the RN version?

We're currently working on a TS specific sprint. Expect lots of TS changed over the next few weeks. Right now, I cant recommend anything, because its likely everything will change.

When we release the new TypeScript work, I'll post an update here!

@csantos-nydig
Copy link

FYI: I believe this is the TS PR Pedro referred to: #659 👀

@Temzasse
Copy link
Contributor

Uuuh nice! 👍🏻 I'll take a look 👀

I actually managed to get the TypeScript part working with the current typings from Stitches repo by copy-pasting them into my repo and modifying the types to work with React Native.

I still need to write some docs for React Native specific stuff and test the lib more (and maybe write some actual tests 🙈) and then I would be ready to publish the first version of stitches-native so that people can try it out 🙂

@jonathantneal
Copy link
Contributor

The TS sprint was a success and we have since launched v1.0.0. Does this work for stitches-native, @Temzasse. Should we close this?

@csantos1113
Copy link
Contributor

Hey @jonathantneal - do you have any near plans to support react-native as part of modulz directly? stitches-native from @Temzasse is a really good starting point, but would be nice to have support for both out of the box!
imagine: @stitches/core , @stitches/react, @stitches/react-native 😍

@Temzasse
Copy link
Contributor

Temzasse commented Sep 1, 2021

@jonathantneal I haven't had the time to incorporate the updated typings from v1 yet but I'll try to get it done as soon as possible. There are a few other smaller things that also need to be implemented in stitches-native but we should be quite close to the initial release 🤞🏻

Maybe we can keep this issue open until the first proper release of stitches-native is out? (there is a pre-release out already so that I was able to reserve the name on npm 😄)

@nandorojo
Copy link

@Temzasse one issue I see with the implementation is that it doesn't use dynamic window dimensions for media queries. Since RN doesn't have native breakpoints, this would be required via a hook. It's what we use for Dripsy. As a result, changing screen size won't update styles. Have you considered a way around this?

@Temzasse
Copy link
Contributor

Temzasse commented Sep 9, 2021

Stitches Native TS update

@jonathantneal I got the new Stitches v1 types working in Stitches Native apart from one issue in RN specific ThemeProvider that I haven't yet figured out (slapped an any to it for now since it doesn't really affect the DX).

There is a new pre-release version that people can try out: npm install stitches-native@canary

Next steps

I still need to comb through the Stitches API and verify that I'm not missing anything important in Stitches Native.

One thing that I know is still missing is the composability of css and styled thingies, eg:

const sharedCssA = css({ /* styles */ });
const sharedCssB = css({ /* styles */ });

const CompA = styled('View', sharedCssA, sharedCssB);
const CompB = styled('View', CompA);

Stitches Native currently only supports the simple version of composing styled components:

const CompA = styled('View', { /* styles */ });
const CompB = styled(CompA, { /* styles */ });

Not sure if it's enough that you are able to spread as many css objects inside a styled object 🤔

const cssA = css({
  backgroundColor: '$background',
});

const cssB = css({
  alignItems: 'center',
  justifyContent: 'center',
});

const Comp = styled('View', {
  flex: 1,
  ...cssA,
  ...cssB,
});

I guess the drawback is that you can't really use variants etc since the last one would overwrite the previous ones.

@Temzasse
Copy link
Contributor

Temzasse commented Sep 9, 2021

@nandorojo could you open an issue about this in the Stitches Native repo so that we could discuss this topic there? 🙂

@Temzasse one issue I see with the implementation is that it doesn't use dynamic window dimensions for media queries. Since RN doesn't have native breakpoints, this would be required via a hook. It's what we use for Dripsy. As a result, changing screen size won't update styles. Have you considered a way around this?

@jonathantneal
Copy link
Contributor

I’m unsure if we should leave this open, direct the discussion to stitches-native, or seek to merge stitches-native into our repository. I’d like to hear your ideas, @Temzasse, before knowing what action to take here.

@jonathantneal jonathantneal added the awaiting feedback Awaiting feedback label Sep 9, 2021
@nandorojo
Copy link

@Temzasse will do. One other suggestion is not not allow the styled function to accept strings. This would break tree shaking, causing of all of RN to import, especially on web.

@LucasUnplugged
Copy link
Contributor

I’m unsure if we should leave this open, direct the discussion to stitches-native, or seek to merge stitches-native into our repository. I’d like to hear your ideas, @Temzasse, before knowing what action to take here.

My 2 cents: if the core Stitches team is willing to merge stitches-native, that might be less responsibility on one person's shoulders, and probably better for consumers of the lib as well.

Can't wait to try Stitches with RN, either way! Thanks @Temzasse !

@axeldelafosse
Copy link

Yes! I think it's a good idea to merge. Thank you for considering it!

@Temzasse
Copy link
Contributor

Temzasse commented Sep 9, 2021

I’m unsure if we should leave this open, direct the discussion to stitches-native, or seek to merge stitches-native into our repository. I’d like to hear your ideas, @Temzasse, before knowing what action to take here.

I'm definitely open to any of the above 👍🏻

@LucasUnplugged had a good point that merging stitches-native to the main Stitches repo would help reduce some of the maintenance burden from me, and in the future it would be easier to have both Stitches for Web and Stitches for React Native evolve hand in hand without too much divergence.

However, I'm also happy keeping stitches-native as a separate project and you can direct any discussion to stitches-native repo. In any case It would be cool to get more contributors aboard in order to make it a more community driven effort 🙂

Here are some pros and cons for the merging idea that came to my mind:

Pros:

  • Better documentation (if RN specific docs can be included in stitches.dev)
    • Also better discoverability since Stitches is already making a name in the CSS-in-JS land
  • More trust from devs (official solution vs. 3rd party solution)
  • Keeping TS types up-to-date would be much easier (this is a big one for me 😄)
  • Possibility to get help with the implementation from the Stitches core team

Cons:

  • Forcing Modulz to take responsibility of something that is not part of their core product (I'm under assumption that you don't do any RN stuff?)
  • Burdening Modulz with RN specific issues that would make the maintenance of stitches more challenging and laborious

@Temzasse
Copy link
Contributor

Stitches Native update

The way forward

@jonathantneal have you and the rest of the team had the opportunity to think about the idea of merging Stitches Native into the Stitches repo?

I feel like Stitches Native is now ready for wider usage. There are still things to do like writing tests (🙈) and refining the docs but the core functionalities should be pretty much in place (unless I have missed something).

One challenge that I have is that I'm not currently able to dog food Stitches Native at my company or in side project so it's difficult to make sure that there are no unhandled edge cases. I'm considering publishing 0.1.0 version (or maybe even 1.0.0) on npm to encourage people to install and try out Stitches Native.

@mtt87
Copy link

mtt87 commented Oct 12, 2021

Interesting thread, I'm building a web components library for my company using Stitches but we also have a React Native app and today I was facing the question: will I be able somehow to reuse the styles for React Native?

@Temzasse I think you should publish the library as 0.1.0 and I think next week I will be happy to try it on our React Native codebase. It's a very small app but happy to test and see if I find issues 😄

@mtt87
Copy link

mtt87 commented Oct 20, 2021

As mentioned I started testing it and I found a few issues with types that I'm reporting here: Temzasse/stitches-native#20

For visibility in case Modulz team wants to help solve them 🙏

@mtt87
Copy link

mtt87 commented Oct 20, 2021

After a couple of hours working with stitches-native these are my findings, bearing in mind that I'm not blaming anyone and in the past I tried myself to implement styled-system utility props with the React Native Stylesheet implementation and it's a difficult task due to the many difference in React Native vs Web.

Anyway here is what I think as of today:

  • typings which is one of the greatest feature of Stitches (if not the best) doesn't seem to work properly, this has a very negative impact in my opinion as it doesn't help catch bugs and perhaps the opposite, help introduce bugs making you feel safe about something that's not. Unlike the web where a property incorrectly written doesn't produce any effect, in React Native if you use fontWeight: 700 it will throw an error crashing the entire app (the correct way is fontWeight: '700' as a string).
  • React Native has a different way of handling various things, for example line height has a total different behaviour, if you use lineHeight: 1.5 on the web it will use a 150% line height based on the font size of that element, while on RN it will use exactly 1.5 so you need to do something like lineHeight: theme.fontSizes[2].value * 1.5.

These 2 points should be fixable but it's not an easy task and it would require a lot of tests.

Now, the most problematic issue: React Native doesn't support nesting/selectors so it makes it really hard or impossible to build components using the composability that Stitches offers (nested Stitches component selectors for example)

You can see it already by trying to build a simple <Button> with variants.
It can be tricky as you need to separate style the "button wrapper" from the "button text" as React Native doesn't support text outside a <Text> component and most of all, doesn't have any concept of "cascade" so each element needs to be styled.
This means you need to declare the N variants 2*N times basically and/or require the developer to remember what are the "correct" combinations = error prone in my opinion
or
if you wanna solve this you need to create a <Button> component with a lot of if variant X && size Y && color Z && state S etc basically making the compound variants offered by Stitches useless.

https://github.com/Temzasse/stitches-native/blob/main/example/src/Example.tsx#L140-L226

In conclusion, I think Stitches is great because of the extensive work they have done around the library to make it work perfectly with the web platform and the React workflow for a developer.
With React Native it feels like just trying to port the library 1:1 is not gonna play very well or falsely suggest to developers that they can port their stitches theme config and components to RN without much effort, when the reality is quite the opposite 😢

@nandorojo
Copy link

typings which is one of the greatest feature of Stitches (if not the best) doesn't seem to work properly, this has a very negative impact in my opinion as it doesn't help catch bugs and perhaps the opposite, help introduce bugs making you feel safe about something that's not. Unlike the web where a property incorrectly written doesn't produce any effect, in React Native if you use fontWeight: 700 it will throw an error crashing the entire app (the correct way is fontWeight: '700' as a string).

If it helps, I ran into this types issue with Dripsy (which is based on Theme UI), and I fixed it on v3.

Screen Shot 2021-10-20 at 12 30 49 PM

See this, this, and docs

I basically use the RN type only, unless you specify otherwise in your theme, which indicates that you have a value in your theme that would override it.

@mtt87
Copy link

mtt87 commented Oct 20, 2021

OT: @nandorojo I didn't know your library, looks interesting I'll check it out 😄 thank you

@Temzasse
Copy link
Contributor

@mtt87 Thank you for your feedback 🙏

Here's my response to the various topics:

TypeScript

  • The types are definitely the hardest part of the library implementation and there is definitely improvements to be made. However, I think it's important to note that the way theme tokens are referenced in Stitches via magic strings is inherently unsafe (unless you use the theme object directly) so I feel like that is a tradeoff that one has to just accept if you want to take advantage of the "magic".
  • Fixing React Native specific issues like the fontWeight (which can cause hard crashes) is definitely something we want to fix, but I don't think handling things like lineHeight difference between Web and Native is the concern of the library. There are other platform differences such shadows that also need to be handled differently so my view is that those kind things should be left to the user to handle (for shadows Stitches Native already provides a utils based solution example in the docs).

Nesting/selectors

  • This is also another limitation that I think just has to be accepted. I understand the pain of not being able to style child components within the parent via selectors - I have many times wished that it would be possible 😁
  • You are right that the power of compound variants is not as great in RN as it is in the Web. Did you have any ideas how we could make this better and more suitable for RN? I wouldn't want to diverge from the Stitches API too much but if there is a much better way to handle some aspect it might make sense to create a RN specific API for it.

In a perfect world there would be an universal styling library that would map 1-1 between Web and Native but I think that is not practically feasible due to the inherent differences between the platforms. I totally understand that when creating design systems / component libraries it would be nice to share as much as possible but I feel like the sharing part should mostly happen at the design token level. Implementing shareable components will probably lead to an implementation that is not optimized for either platform. Sorry if this sounds harsh - I just wanted to share my philosophy about this subject 😅

@mtt87
Copy link

mtt87 commented Oct 21, 2021

Thanks for your reply, and don't worry it didn't sound harsh at all 😄
You are right that there are unfortunately some trade-offs that we can't avoid.

I'll think about if there are ways to improve the compound variants but I suspect there aren't 😭

I'll have a look at the source code to see if I can help with the types perhaps although I'm not a TS expert I'd say 😄

@nandorojo
Copy link

nandorojo commented Oct 21, 2021

I like the discussion here.

Here’s my take: we should embrace the way that React Native works, rather than fit it to be like CSS.

Nested selectors and pseudo elements give you convenience, but don’t scale well for a component-based design system. I think it’s actually a good limitation of React Native to not have these.

I recommend this talk from the creator of React Native for Web, it really solidified that perspective for me: https://youtu.be/tFFn39lLO-U

I’ll also be talking about this topic at Next.js Conf in a few days: https://nextjs.org/conf/speakers/fernando

In a perfect world there would be an universal styling library that would map 1-1 between Web and Native but I think that is not practically feasible due to the inherent differences between the platforms.

I agree, but the baseline for these styles should be React Native’s styling system, not CSS. If that’s the case, then we can in fact get the 1-1 mapping. React Native’s styling system lends itself to work well on web. CSS, on the other hand, is too low level and coupled to HTML to be used on other platforms.

I’m not sure what the right answer is in the case of porting Stitches and its variants, but I think we can take the best of stitches, and still lean into React Native (+ Web)’s styling philosophy.

I came to this conclusion after using Dripsy in production for beatgig.com for about a year, where my original goal was also to copy all the CSS options from theme UI.

I hope that makes sense.

@mtt87
Copy link

mtt87 commented Oct 21, 2021

Here’s my take: we should embrace the way that React Native works, rather fit it to be like CSS

I share your same feeling here

I recommend this talk from the creator of React Native for Web, it really solidified that perspective for me: https://youtu.be/tFFn39lLO-U

I love this talk from Nicolas, watched many times 😄

I’ll also be talking about this topic at Next.js Conf in a few days: https://nextjs.org/conf/speakers/fernando

Sounds great I didn't know, looking forward to watch you and @peduarte talk!

I’m not sure what the right answer is in the case of porting Stitches and its variants, but I think we can take the best of stitches, and still lean into React Native (+ Web)’s styling philosophy.

Agree I think the "design token first" approach of stitches and the DX around it provided by perfectly crafted Typescript types is super valuable.

Perhaps @Temzasse should consider making stitches-native a stripped version of stitches with only the parts that make sense in RN.
For example considering to totally remove the compound

I came to this conclusion after using Dripsy in production for beatgig.com for about a year, where my original goal was also to copy all the CSS options from theme UI.

One observation here: I loved rebass / styled-system / theme-ui in the past but I find the array syntax to be more error prone compared to the breakpoints you declare in stitches "@lg": { ... }, "@sm": { ... }

@nandorojo
Copy link

One observation here: I loved rebass / styled-system / theme-ui in the past but I find the array syntax to be more error prone compared to the breakpoints you declare in stitches "@lg": { ... }, "@sm": { ... }

yeah, I’ve considered this. I might add this object functionality in v4.

@Temzasse
Copy link
Contributor

@jonathantneal can I ask a TypeScript related question? 🙂 It's related to the feedback from @mtt87 and the notion of narrowing types like fontWeight to values that are correct in React Native.

Is it possible to restrict certain CSS properties to only accept either a string that starts with $ (indicating it's a token) or a specific union (eg. 'normal' | 'bold' | '100' etc.)?

By looking at the types in css-util.d.ts I've deducted that by having Util.Index in here:

{
  [K in keyof CSSProperties]?:
    | ValueByPropertyName<K>
    | TokenByPropertyName<K, Theme, ThemeMap>
    | ThemeUtil.ScaleValue
    | Util.Index // <------------------------
    | undefined
}

leads to any number being a valid value eg. when doing:

const Comp = styled('Text', {
  fontWeight: 700,
});

I'm not sure I understand the need for Util.Index here 🤔 Could you help me understand why it's one of the types in the union?

By removing Util.Index from the union I'm able to fix the fontWeight typing issue but I'm afraid I'm overlooking something that will break if I remove that type from the union.

Addiotionally, removing Util.Index doesn't really solve string value based issues, eg. when doing something like this:

const Comp = styled('View', {
  alignItems: 'not-valid',
});

So I guess the main question is this: is there a way to make these types more strict without losing the nice autocomplete feature for tokens? Unlike on the Web an invalid CSS value can cause a hard crash in React Native so it would be very helpful if we were able to protect the users of Stitches Native from these kind of errors when using TypeScript.

Thanks in advance!

@mtt87
Copy link

mtt87 commented Oct 23, 2021

I didn't look at the codebase but all these styling properties are already correctly typed
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react-native/index.d.ts#L932
Shouldn't stitches-native somehow extend these types?

@Temzasse
Copy link
Contributor

Temzasse commented Oct 23, 2021

Hmm I might have figure this out myself 😄

If I remove Util.Index type from the union that I mentioned in my earlier comment and then also remove the literal union type thingy (string & {}) (which enables autocompletion while still allowing any string as a value) from all CSS properties, eg:

type FontWeightProperty =
  | 'normal'
  | 'bold'
  | '100'
  | '200'
  | '300'
  | '400'
  | '500'
  | '600'
  | '700'
  | '800'
  | '900'
  | (string & {}); // <-------- remove this

then it's possible to narrow down the types to only acceptable value from React Native's perspective.

Now I'm only wondering what are the side effects of this change 🤔

All three main use cases seem to work fine:

  1. Valid CSS values
const Comp = styled('Text', {
  fontWeight: '700',
});
  1. Theme token magic string
const Comp = styled('Text', {
  fontWeight: '$bold',
});
  1. Reference theme token directly
const Comp = styled('Text', {
  fontWeight: theme.fontWeights.bold,
});

@jonathantneal is there something that I'm missing?

@Temzasse
Copy link
Contributor

@mtt87 I have released a new canary version (v0.0.1-6) where the CSS property types are now more strict.

If you (and other people too) could test the new version I would appreciate that a lot 😊

One thing that I did not change yet but was thinking about is the type of the theme definition. Currently it looks something like this:

export type Theme<T = {}> = {
  borderStyles?: { [token in number | string]: boolean | number | string };
  borderWidths?: { [token in number | string]: boolean | number | string };
  colors?: { [token in number | string]: boolean | number | string };
  fonts?: { [token in number | string]: boolean | number | string };
  fontSizes?: { [token in number | string]: boolean | number | string };
  fontWeights?: { [token in number | string]: boolean | number | string };
  letterSpacings?: { [token in number | string]: boolean | number | string };
  lineHeights?: { [token in number | string]: boolean | number | string };
  radii?: { [token in number | string]: boolean | number | string };
  sizes?: { [token in number | string]: boolean | number | string };
  space?: { [token in number | string]: boolean | number | string };
  zIndices?: { [token in number | string]: boolean | number | string };
}

I'm wondering if we should narrow the types here too for borderStyle and fontWeights which have a limited set of acceptable values 🤔

Currently you can do this (which is invalid):

const { styled } = createStitches({
  theme: {
    fontWeights: {
      bold: 700,
    }
  }
});

const Comp = styled('Text', {
  fontWeight: '$bold',
});

Again I'm not quite sure if there are some side effects for this change 😅

@mtt87
Copy link

mtt87 commented Oct 25, 2021

@Temzasse is stitches-native using the underlying RN Stylesheet.create() or is it just inlining styles?
@nandorojo is dripsy using the underlying RN Stylesheet.create() or is it just inlining styles?

In theory using the Stylesheet.create() should always be the preferred way although for simple apps shouldn't make any noticeable difference.

@jonathantneal
Copy link
Contributor

Re: TS font-weight supporting any number.

The CSS font-weight property supports any number in the range of 1 to 1000, which users of Variable fonts will expect and/or need. TypeScript does not yet support numerical ranges that I am aware of at the time of this writing, and so "any number" becomes the least complex way to support this.

Re: TS font-weight supporting any string.

The CSS font-weight property supports any string to openly support var(), calc(), env() or any other valid value.

Re: Narrowing font-weight to what React Native supports.

If React Native is so far behind the abilities of CSS, that’s unfortunate. It may also be inevitable if React "Native" is not truly CSS. I would expect these platforms to continue to evolve separately. It seems like attempting to bridge browsers and 'native' by their weakest link requires rethinking what 'styles' are in a very narrow way.

(The closest similar leap I can think of would be supporting IE6, which did not support chaining. It sounds very miserable / profitable.)

@nandorojo
Copy link

@nandorojo is dripsy using the underlying RN Stylesheet.create() or is it just inlining styles?

Yes, Dripsy uses StyleSheet.create. Dripsy also efficiently deep compares your sx prop, which means you can write your styles in render.

Keep in mind that StyleSheet.create is a web-only optimization. On native, it actually is a function that returns its argument and provides TS convenience.

@mtt87
Copy link

mtt87 commented Oct 25, 2021

Keep in mind that StyleSheet.create is a web-only optimization. On native, it actually is a function that returns its argument and provides TS convenience.

I'm aware on the web (React Native Web) using the StyleSheet.create() function allows the CSS-in-JS to properly be handled in terms of atomic css generation, otherwise it stays as inline styling with no optimization but if I understand correctly this is also an optimization in React Native (mobile).

According to the docs and other answers I found online, the StyleSheet.create() function basically sends the style to the bridge and creates IDs so that further implementations of those styles are optimized and they don't have to be sent through the bridge again.

https://reactnative.dev/docs/stylesheet#flatten

IDs enable optimizations through the bridge and memory in general. Referring to style objects directly will deprive you of these optimizations.

Maybe it's not noticeable in very small simple apps but it might be in more complex apps

@nandorojo
Copy link

StyleSheet.create does nothing on native. Look at the source code: https://t.co/xkjnyIWOXl

@mtt87
Copy link

mtt87 commented Oct 25, 2021

StyleSheet.create does nothing on native. Look at the source code: https://t.co/xkjnyIWOXl

Oh wow interesting! This is confusing online there are different places including the documentation that don't seem to be very clear about this but the code is the ultimate source of truth. Thanks!

@justindomingue
Copy link

@peduarte any official update from the dev team on this ?

@hadihallak
Copy link
Member

Hey everyone 👋
We don't currently have plans to add official react-native support.
@Temzasse The stitches-native does look very promising and I really like how well you're documenting the API so I'd be very happy to merge a PR to add it to the list of community built packages

@Temzasse
Copy link
Contributor

Temzasse commented Jun 6, 2022

@hadihallak here's the PR 😉

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

No branches or pull requests