Closed zRelux closed 2 years ago
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?
@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!
FYI: I believe this is the TS PR Pedro referred to: https://github.com/modulz/stitches/pull/659 π
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 π
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?
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
π
@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 π)
@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 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
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.
@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?
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.
@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.
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 !
Yes! I think it's a good idea to merge. Thank you for considering it!
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:
Cons:
css
and styled
@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.
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 π
As mentioned I started testing it and I found a few issues with types that I'm reporting here: https://github.com/Temzasse/stitches-native/issues/20
For visibility in case Modulz team wants to help solve them π
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:
fontWeight: 700
it will throw an error crashing the entire app (the correct way is fontWeight: '700'
as a string).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 π’
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.
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.
OT: @nandorojo I didn't know your library, looks interesting I'll check it out π thank you
@mtt87 Thank you for your feedback π
Here's my response to the various topics:
TypeScript
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
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 π
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 π
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.
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": { ... }
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.
@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!
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?
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',
});
2) Theme token magic string
const Comp = styled('Text', {
fontWeight: '$bold',
});
3) Reference theme token directly
const Comp = styled('Text', {
fontWeight: theme.fontWeights.bold,
});
@jonathantneal is there something that I'm missing?
@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 π
@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.
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 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.
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
StyleSheet.create
does nothing on native. Look at the source code: https://t.co/xkjnyIWOXl
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!
@peduarte any official update from the dev team on this ?
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
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