Feature request: Be able to control the class load order when merging classNames

[menelike]

Version: 4.1.1
Environment: Both
Type: Feature

Description

When fela is used in context of react, the lazy evaluation results in styles getting appended to stylesheet in an arbitrary order. If classname is inherited from a parent and merged within the child component (className={'${this.props.className} ${baseClassName}'}) there is no predictable way to tell which className wins.
As a result the rendered result can diverge based on component load order and is hardly testable.

More details here (Non-deterministic Resolution)

Current solutions

1. use style, avoid className

Problems:

• You always need to look at the child's style implementation to predict the behaviour.
• More complexity in conjunction with shouldComponentUpdate (shallowCompare(this.props.style, nextProps.style))
• No way to pass style and className in parallel

2. Use specific style props

Problems:

• a child must define flex: 1 1 auto, a parent must use flexGrow: 0
• does not work on all props (width, height, etc.)

3. Use combineRules

Problems:

Try to avoid combineRules if possible. If you are using combineRules excessively you might have to rethink and refactor your styling technique to achieve a more modular and simple one.
combineRules will always create a new rule which prevents using already cached sub-rules. Therefore it might negatively affect the rendering performance if you are using multiple combined rules.

4. Use an index when inserting the stylesheet (current solution by jss)

Without going into the details: it's ugly, it's a workaround, it's a sword of Damocles.

Goals

A child's API should stay consistent to DOM elements.

If you pass style, you expect that (merged or replaced) style on the child, if you pass className you expect (merged or replaced) className on the child.

You never need to expect some black magic where stylebecomes className

Proposal

simple POC

It caches className and it's corresponding propType e.g. {a: 'flex', b: 'width, c: 'width' }

It can then be used like

// a = () => {flex: '1 1 auto'}
// b = () => {width: 100}
// c = () => {width: 200}

// b and c use the same prop
renderer.mergeClassNames('a b c') 
// expected className 'a c' (last wins)

[robinweser]

Sounds like this would fit into an enhancer perfectly. Yet I guess we have to rewrite the combineRules part. It's not true anymore. Combining now actually is probably the way to go.

[robinweser]

I will release some new tools and guides on how to actually compose rules / components with Fela, which makes it obsolete to use any of the above requested addons.

Teaser: All it uses is combineRules and it works great :)

[menelike]

Great, just came back from holiday and wanted to ask if PRs are welcome.

[robinweser]

Any thoughts and ideas are still welcome. But please share on Gitter before spending a lot of time implementing stuff that might not be added at all, hehe :P I am curious on your ideas though

[robinweser]

With the next release, I got you covered. Locally, createComponent and ?connect now automatically compose their rules.

[menelike]

Thanks for solving this request @rofrischmann !

I could not find the right commit to preview the upcoming API (and logic) behind the solution. Could you be so kind and mention the code/commit/doc?

[robinweser]

I guess it's all here:
https://github.com/rofrischmann/fela/blob/master/modules/bindings/react/createComponent.js#L21

Documentation is coming soon. (tomorrow hopefully)

[menelike]

Great, thanks a lot! (btw. slick solution)