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

Performances #444

Closed
gregberge opened this issue Mar 31, 2019 · 4 comments · Fixed by #513
Closed

Performances #444

gregberge opened this issue Mar 31, 2019 · 4 comments · Fixed by #513

Comments

@gregberge
Copy link

Hello, first I would like to thank you for this project. It was a source of inspiration for me, the goal of this issue is to improve this library and make it the default and best solution for styling components.

Problem

I would like to use styled-system in Smooth UI instead of using my own system. One of the problem is performance, it was one of my concern when I decided to create my own system, and it is still my concern.

As you can see in this benchmark, Smooth UI system is twice faster than styled-system:

@smooth-ui/system x 466,611 ops/sec ±0.65% (89 runs sampled)
@material-ui/system x 239,762 ops/sec ±0.44% (84 runs sampled)
styled-system x 213,444 ops/sec ±0.59% (88 runs sampled)
Fastest is @smooth-ui/system

https://repl.it/@neoziro/system-benchmarks
https://github.com/smooth-code/smooth-ui/blob/master/benchmarks/system.js

Solution

The problem in your solution is that it iterates on styles instead of iterating on props. There is always more styles than props and if we want to support more CSS properties, it will grow up.

Another problem is the lack of style indexing, by indexing styles on properties, you are always flat and very effective.

The best is to give a look to the compose function in smooth-ui/system.

@jxnblk
Copy link
Member

jxnblk commented Mar 31, 2019

Thanks for putting this together! Performance is certainly something that could be improved in styled-system, and if that's one of the main deterrents for you and possibly others, I'd love some help in trying to make that better!

If you have time to start a PR (even if just a spike), I think that'd be a great way to see the difference.

The problem in your solution is that it iterates on styles instead of iterating on props.

Yeah, I think a very early prototype did loop over the props for space, but wanted to move the API to use more of a composed approach. If you have a way to make the implementation better with the current style & compose APIs, I'd love to see that. (I'll also poke around the smooth-ui repo later)

As a side note, I think we'll want to keep an eye on the API in @styled-system/css, which (currently) loops over full style objects as well (allowing styled-system props to work inside nested selectors like &:hover)

@gregberge
Copy link
Author

Good to hear! I will try to work on a PR in this way! I don't have so much time but I will try.

@peduarte
Copy link

peduarte commented Apr 8, 2019

This is awesome! Happy to help out 👋

I'm using Styled System a lot, and performance was also one of my concerns, so I'd love to help make it better/faster/stronger 💪

@gregberge
Copy link
Author

I will submit a PR, as soon as I have a moment!

gregberge added a commit to gregberge/styled-system that referenced this issue Apr 28, 2019
Following styled-system#444, I bring perf improvements from @smooth-ui/system into styled-system.
The goal is to make this library the best one and the universal one.

What have changed?

- I switched to the core of @smooth-ui/system, more performant
especially with multiple compose. Composition is now flat, it means you
can compose 10 times and it will not be slower. We are probably in O(n),
where "n" is the number of props.
- I splitted the logic in several files to make it clearer
- I switched to rollup in order to optimize build, provide umd and
improve tree shaking

What are the breaking changes?

- We now rely on the order of props. ES2015 is here and we can rely on
it, see this tweet for context: https://twitter.com/ianstormtaylor/status/1115680037875752963.
- We are more strict in breakpoints, we don't accept a non-existing
breakpoint
- We don't export utilities like get, themeGet, etc...
- PropTypes are no longer included, they can be generated from meta
- We don't return array, just an object with all props merged
- Properties that default to pixels unit (like margin, padding) no
longer include it
gregberge added a commit to gregberge/styled-system that referenced this issue Apr 28, 2019
Following styled-system#444, I bring perf improvements from @smooth-ui/system into styled-system.
The goal is to make this library the best one and the universal one.

What have changed?

- I switched to the core of @smooth-ui/system, more performant
especially with multiple compose. Composition is now flat, it means you
can compose 10 times and it will not be slower. We are probably in O(n),
where "n" is the number of props.
- I splitted the logic in several files to make it clearer
- I switched to rollup in order to optimize build, provide umd and
improve tree shaking

What are the breaking changes?

- We now rely on the order of props. ES2015 is here and we can rely on
it, see this tweet for context: https://twitter.com/ianstormtaylor/status/1115680037875752963.
- We are more strict in breakpoints, we don't accept a non-existing
breakpoint
- We don't export utilities like get, themeGet, etc...
- PropTypes are no longer included, they can be generated from meta
- We don't return array, just an object with all props merged
- Properties that default to pixels unit (like margin, padding) no
longer include it
gregberge added a commit to gregberge/styled-system that referenced this issue May 5, 2019
Following styled-system#444, I bring perf improvements from @smooth-ui/system into styled-system.
The goal is to make this library the best one and the universal one.

What have changed?

- I switched to the core of @smooth-ui/system, more performant
especially with multiple compose. Composition is now flat, it means you
can compose 10 times and it will not be slower. We are probably in O(n),
where "n" is the number of props.
- I switched to rollup in order to optimize build, provide umd and
improve tree shaking

What are the breaking changes?

- We now rely on the order of props. ES2015 is here and we can rely on
it, see this tweet for context: https://twitter.com/ianstormtaylor/status/1115680037875752963.
- Object keys or array keys have now equivalent parsing
gregberge added a commit to gregberge/styled-system that referenced this issue May 12, 2019
Following styled-system#444, I bring perf improvements from @smooth-ui/system into styled-system.
The goal is to make this library the best one and the universal one.

What have changed?

- I switched to the core of @smooth-ui/system, more performant
especially with multiple compose. Composition is now flat, it means you
can compose 10 times and it will not be slower. We are probably in O(n),
where "n" is the number of props.
- I switched to rollup in order to optimize build, provide umd and
improve tree shaking

What are the breaking changes?

- We now rely on the order of props. ES2015 is here and we can rely on
it, see this tweet for context: https://twitter.com/ianstormtaylor/status/1115680037875752963.
- Object keys or array keys have now equivalent parsing
@jxnblk jxnblk mentioned this issue Jun 2, 2019
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants