Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Convert components to use LESS compiled into style attrs #149

Closed
wants to merge 2 commits into from

Conversation

highruned
Copy link

Status: Opened for discussion
Reviewers: None yet

Notes

After having weighed the options, I felt like this deserved some exploration. The demos are using global CSS, which kind of goes away from the same idea for proposing HTML in JS. Then there's some which seem to be compiling and injecting style tags on a per-component basis or something (seems like Instagram). That's where vjeux comes in with inline styles. Which generally I like because we have interoperability with other interfaces (React Native now, maybe games or wearables in the future - Unreal Engine, Hololens). Whichever way Facebook moves, I feel like there's going to be a demand for some sort of utility library around styling. Things like vendor prefixing, validating properties, and generally just making it easier to style. If we have a system around something that's already quite mainstream like LESS, it should be easier to adopt. While this system may not be as fast as straight style objects, it still compiles down into the same thing. I'm not sure the performance is going to be noticeably impacted. It needs to be explored, as I know UI performance is one of the goals of React people. I'd love to explore this some more with somebody :)

I did try transforming the string into an AST in place, but it was too large and required some work hooking up the right classes. Similar to GraphQL, I gather.
There is actually some additional magic going on here — our transform pipeline actually embeds the AST directly (as JavaScript objects) in place of the template — but generally developers won't need to worry about that at all.

TODO

  • Clean up LESS (actually just CSS right now -- haven't converted to take advantage of LESS yet)
  • Style attributes should be optimized (stop returning 4 attributes for each border).
  • Helper so we can just go styles.find('#todo-list li.edit') and it'll return all rules that match that selector, including #todo-list li.edit {}, #todo-list li {} and li {}.

Changes

  • Moved the global CSS into each relevant component.
  • Added 6to5ify so we can use new strings (for CSS/LESS).
  • Added LESS so we can parse the string into an AST tree.
  • Added JSCSSP so we can parse the resulting CSS from the AST tree into style rules without the browser.

How to test

  • Start the demo with npm start after npm install and you should see something like this:
    preview

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@fisherwebdev
Copy link
Contributor

This seems like a fine thing for you to do in your own fork, but it really doesn't belong here. The Flux-TodoMVC app should be as simple as possible, with a minimum of clutter. It's not supposed to be an ideal implementation. It's supposed to be instructional for someone new to Flux.

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

Successfully merging this pull request may close these issues.

3 participants