Skip to content
This repository has been archived by the owner on Jul 19, 2019. It is now read-only.

Add April 28 notes #10

Merged
merged 1 commit into from
Apr 28, 2016
Merged

Add April 28 notes #10

merged 1 commit into from
Apr 28, 2016

Conversation

gaearon
Copy link
Member

@gaearon gaearon commented Apr 28, 2016

Feel free to discuss in this PR!

@gaearon gaearon merged commit d1d1b15 into master Apr 28, 2016
@gaearon gaearon deleted the gaearon-patch-1 branch April 28, 2016 20:34
@rtsao
Copy link

rtsao commented Apr 28, 2016

I'm interested in details regarding the implementation of StyleSheet.create() used by @vjeux and more specifically, the mentioned issues concerning "retaining correct specificity". What problems were encountered?

@gaearon
Copy link
Member Author

gaearon commented Apr 28, 2016

We’ll share more details when we’re confident this approach isn’t flawed in some fundamental way. We don’t want to say “hey, we now use StyleSheet.create on the web!” and have everybody pick it up before realizing that there are problems we can’t solve, and that we have to give up on it.

The issues with specificity are the same ones that pretty much any library that tries to translate inline styles to CSS sheets encounters at some point. I don’t have very good understanding of this but maybe @vjeux (he’s working on it), @ianobermiller (author of Radium), @andreypopp (author of react-stylesheet), or @kof (author of jss) could share some of their expertise.

@kof
Copy link

kof commented Apr 28, 2016

I am not sure I am excited about StyleSheet.create heading in reacts core. It makes react more opinionated. A huge amount of things can be done in this area. It's not just simple js->css compilation. With jss and it's plugins I am trying to build the postcss for css in js.

I would rather strive to make react smaller and more focused.

As for specificity I can say: don't use more than one class on an element, so you won't get any specificity issues (except of class vs. inline styles but this one is obvious). We can always compose everything one rule needs using js objects and then render it to just one rule and use just one class name.

@vjeux
Copy link

vjeux commented Apr 28, 2016

There is no plans for StyleSheet.create to be inside of React core at this point fyi. The changes in React I'm considering are better support for inline styles such as RTL support by swapping left and right, browser prefixing, structured way to handle transform attribute instead of a string...

Right now what I'm working on is building a StyleSheet.create variant mostly compatible with the one in React Native to output corresponding CSS. This mostly involves fb build infra and doesn't touch React at all

@kof
Copy link

kof commented Apr 28, 2016

I am curious if the opposite would also make sense: using things like media queries or animations from CSS in react-native.

@sophiebits
Copy link
Member

I'll also add: the current experimentation is very tied to existing Facebook infrastructure which means that it is easier for us to move to it as an incremental step while maintaining feature parity and performance with our existing CSS bundling solution. This also means that the code and individual techniques wouldn't be widely applicable even if we were to do the work to extract them at this stage. However, the longer-term goal is to move towards relying on the Facebook CSS infrastructure less and driving everything from JavaScript while having strong confidence that the solution is robust and efficient, at which point we'll look at sharing the work with the community.

@rtsao
Copy link

rtsao commented Apr 28, 2016

I was a bit puzzled by specificity problems, since I can't think of any css-in-js libraries that have problems with specificity in particular (at least that I'm aware of).

In my view, there's a couple things that can make React's handling of inline styles difficult:

  • Proper resolution when mixing shorthand and longform CSS properties and values
    • Maybe just disallow shorthand?
  • Vendor prefixed value fallbacks
    • It'd be nice to not need to know right vendor prefix ahead of time
    • There was a hacky way to get this with React 0.14 and below, but no longer works with 15
    • Other than this, inline-style-prefix-all is totally sufficient

Glad to see you're looking at a replacement for transform strings, which can be problematic (see issue with longform/shorthand properties)

Regarding RTL, wouldn't a userland inline-style-prefixer equivalent for RTL suffice (e.g. a style object version of rtlcss)? I'd imagine it'd be reasonably lightweight in terms of bytes (since using style objects should eliminate the postcss dependency) and easy to integrate in a React.createElement wrapper.

@vjeux
Copy link

vjeux commented Apr 28, 2016

RTL isn't as easy as swapping marginLeft to marginRight, you also need to parse margin: 0 5px 2px 3px, transform: translateX(30px). We also horizontally flip all the inline images so that if there are arrows or directions in them, they point to the right place. Now if you are working with images, we have a lot of infrastructure in order to automatically generate image sprites based on which images are being used together.

I hope it gives you a sense of what are the challenges, and many of them are very fb-infra specific at this point.

@ffxsam
Copy link

ffxsam commented Apr 28, 2016

I'm very interested in keeping tabs on what the React team decides in an official capacity what is the best practice for styles in React. It's obviously been a point of contention for quite some time now. I started with JS styles, but abandoned it for SCSS+BEM, and it works for my project, though is bad for portability and sharing things via npm.

@kof
Copy link

kof commented Apr 29, 2016

My idea for fixing "portability" is to keep styles mostly out of components by using a standard for theming, a reference implementation happens right now here

@hnqso
Copy link

hnqso commented Apr 29, 2016

I'm wondering about debugging inline styles. Thinking specifically in the animation tooling on Chrome DevTools or the simple but handy things such as the toggle element classes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants