callstack / linaria

Zero-runtime CSS in JS library
https://linaria.dev
MIT License
11.67k stars 417 forks source link

`fragment` tag for composition #244

Open satya164 opened 6 years ago

satya164 commented 6 years ago

Problem

Currently it's not possible to compose styles from multiple classnames together. Previously we had a include helper which would take a class name and return the unprocessed CSS text. I removed it because it wasn't strightforward to implement it with the new approach and I haven't needed to use it personally. In addition, an extra class name was generated whether you used it anywhere or not.

Another usecase is that once you get into an interpolation inside the tagged template, we don't parse the nested interpolations. So it's not possible to do something like following:

const Box = styled.div`
  font-size: 14px;

  ${borders
    .map(({ p, w }) => `border-${p}: ${w}px solid ${props => props.color}`)
    .join('\n')};
`;

styled-components has exports css tag to support this. However, the css tag is used for class names in Linaria, so we can't do the same.

Prior art

Emotion allows you to interpolate a class name which inlines its styles: https://emotion.sh/docs/composition

One problem with this approach is that you're interpolating a class name, but getting the CSS text instead. This breaks the following use case where you might want to refer to another class name:

const paragraph = css`
  font-size: 14px;
`;

const article = css`
  backgrounc-color: white;

  .${paragraph} {
    font-size: 16px;
  }
`;

Current solution

Currently, you could do something like this:

const shared = `
  font-size: 18px;
`;

const Title = styled.h1`
  ${shared};

  color: black;
`;

There are 2 issues with this approach:

  1. There's no syntax highlighting or autocomplete for the CSS
  2. It's not possibe to use dynamic interpolations like you would in styled

Proposal

We could add a new fragment tag to allow this use case:

const shared = fragment.css`
  font-size: 18px;
`;

const Title = styled.h1`
  ${shared};

  color: black;
`;

The fragment tag itself won't generate any class names, but when the result is interpolated, it will inline the CSS text along with the dynamic interpolations. It'll work very similarly to how styled-components handles interpolation of css tagged template literals. As for parsing, it would be parsed similar to how the styled.x tags are parsed.

Note: Initially I thought of having just fragment, but went for fragment.css since it allows us to have syntax highlighting and autocomplete.

Since the fragment tag won't generate a class name, to use it as a class, you could do something like this:

const title = css`${shared}`;

You could even inline it if you don't like creating an extra variable:

<h1 class={css`${shared}`} />

Why not add it

It requires extra code, especially if we want to support interpolations and don't want to leave the CSS code for the fragment inside the JS bundle.

silvenon commented 6 years ago

Just to check, this route is impossible/unacceptable?

const card = css`
  background: #fff;
  box-shadow:
    0 1px 3px rgba(0, 0, 0, 0.12),
    0 1px 2px rgba(0, 0, 0, 0.24);
`
const article = css`
  font-size: 18px;
  ${card};
`

console.log(card)
// "a123"
console.log(article)
// "b123 a123"
// ☝️it's not a mixin, it just concatenates class names
satya164 commented 6 years ago

@silvenon we cannot do that because then the following will not be correct:

const paragraph = css`
  font-size: 14px;
`;

const article = css`
  background-color: white;

  .${paragraph} {
    font-size: 16px;
  }
`;

Here the desired result was to use different styles for paragraph when nested inside article, but concatenating the class names will mean that all the styles from paragraph will also be applied to article.

You can instead do following:

const card = css`
  background: #fff;
  box-shadow:
    0 1px 3px rgba(0, 0, 0, 0.12),
    0 1px 2px rgba(0, 0, 0, 0.24);
`;

const article = css`
  font-size: 18px;
`;

<div class={cx(card, article)} />
silvenon commented 6 years ago

I thought that it would be possible to detect .${paragraph} { vs ${paragraph}; and act accordingly. Yes, we can concatenate classes with cx, but this way would be much shorter, especially with styled. Also, unlike fragments, concatenating class names wouldn't duplicate styles.

But if that's not possible, fragment API (mixins?) sounds good! Although instead of adjusting API to text editor plugins, we can adjust text editor plugins to API instead? What I mean is that it might be better to use fragment instead of fragment.css (I'd be the first one to submit a PR to styled-components extension for VS Code).

satya164 commented 6 years ago

I thought that it would be possible to detect .${paragraph} { vs ${paragraph}; and act accordingly

It could be possible if we parse the CSS as it's constructed. But then we also have to distinguish between class names being interpolated and plain strings being interpolated. It adds complexity which isn't worth it imo. It's also not straightforward to understand.

Yes, we can concatenate classes with cx, but this way would be much shorter, especially with styled. Also, unlike fragments, concatenating class names wouldn't duplicate styles.

I think the main appeal of composition in emotion is that when you compose 2 classes, you don't have to worry about the cascade, unlike just concatenating 2 class names. In emotion, it'll duplicate the styles to acheive the correct styles, which is what fragment is aiming to do. Also if we make the feature look likewhat emotion does, but just concatenate class names, it might be confusing too. So I think better to be explicit and use cx.

Although instead of adjusting API to text editor plugins, we can adjust text editor plugins to API instead

That'd certainly be preferable, but I highly doubt that they'll accept such a PR to styled-components extension, since styled-components doesn't support such a feature. AFAIK they rejected a PR adding support for css prop which emotion has, because styled-components doesn't support it.

Another problem is that fragment is a very generic name, so it'll be impossible to convince them to highlight it as CSS. For example, another library might export something named fragment, in which case, we wouldn't want to parse that.

otbe commented 5 years ago

Thats really the last missing piece to switch to linaria. I have 2 projects in my mind. In one project we use css modules (with compose) and in the other project we use styled-components. And we do things like:

// generic mixin for margins, especially nice when using TS
const margins = (marginProps) => `margin: ${compute(marginProps)}`;

const Wrapper = styled.div`
  ${margins};
  background-color: green;
`

If I get you correct, this should be possible with fragment.css? Would love to see somthing like this :)

ferdaber commented 5 years ago

What if another function is exported from the library that where you can call it that allows you to detect that the intent is to compose?

import { cx, css, compose } from 'linaria'

const paragraph = css`
  font-size: 14px;
`;

// this composes the other class's styles
const paragraphArticle = css`
  background-color: white;

  ${compose(paragraph)};
`;

// this selects the other class
const article = css`
  background-color: white;

  .${paragraph} {
    font-size: 16px;
  }
`;
satya164 commented 5 years ago

@ferdaber We did exactly the same previously. but it's not very useful because it's not possible to use interpolations etc. and it just becomes a fancier string interpolation. It's also not straightforward to implement.

ferdaber commented 5 years ago

I think the issue with fragment is that it may not be obvious to a user importing it that it's a fragment without looking at the source code. I know with my team we have a lot of "mixins" but some are actually fragments but some are nearly-full styles of components that need to be composed, and it might be a bit of mental overhead to consider when to actually do className={css(myFragment)} vs just className={myFragment}.

Can you elaborate why this method would make it impossible to use interpolations? I think this is an awesome library but this missing feature is something I use very, very often in emotion currently.

satya164 commented 5 years ago

Can you elaborate why this method would make it impossible to use interpolations

Because we can't support dynamic interpolations in the css tag, and will throw a syntax error if you try to use it. This alone makes the use case very limited. We could make it so that only css tag can be composed inside another css tag and only styled tag can be composed inside another styled tag, but it'll be better to have a solution which is interoperable.

it may not be obvious to a user importing it that it's a fragment without looking at the source code

It's a valid argument, but it's fair to assume that if you are importing something, you need to know what it is that you're importing. It's the same in other parts of JS as well.

It comes down to how you organize the code. You can keep fragments in a separate file (like people usually do for sass mixins etc.) to make it super obvious.

when to actually do className={css(myFragment)} vs just className={myFragment}

You should avoid doing className={css(myFragment)} because unlike a runtime solution like emotion, in linaria, this means every time you do that, it'll create a new CSS rule which is basically duplicated code.

ferdaber commented 5 years ago

Ah gotcha, I thought you were talking about any interpolation in general, because I think that css still supports partial evaluation of variables, right? css'${colors.white}'

With css(myFragment) I was referring to your proposal, wouldn't they be equivalent?

<div className={css`${myFragment}`} />
<div className={css(myFragment)} />
brandonkal commented 5 years ago

What if array destruturing was used?

Normally:

const [paragraph] = css`
  color: black;
`

Then to grab the rules for composition within another css or styled tag:

const [paragraph, pRules] = css`
    color: black;
`

const main = styled.main`
  .${paragraph} {
       color: green;
   }

.copy {
  ${pRules}
}
`
sshmyg commented 5 years ago

Will 1.4v get some solution for this issue?

trusktr commented 5 years ago

Emotion allows you to interpolate a class name which inlines its styles:

Unfortunately Emotion devs are removing that feature. The console will throw a red error at you, telling you it that the the feature will be removed in an upcoming version.

jayu commented 4 years ago

Styles composition would be extremely useful once we introduce react-native support #236 We could potentially share more code between react and react-native

jonatansberg commented 4 years ago

We just ran in to a case where we would really benefit from this. A colleague of mine is currently trying to write a codemod for migrating ~50 separate apps (roughly 10 000 components) from Emotion 9 to Linaria, and this is a pattern thats used in a lot of them.

Would it be possible to keep the current behaviour of css-tags serialising to classNames (e.g. having .toString return a class name string) and then enable css-tag composition by either:

a) overriding the .toJSON method to return a css-string or object litteral, or

b) use the same kind of "sniffing" thats implemented here to get the rules for css-tags and the class name for styled components?

jayu commented 4 years ago

@jonatansberg your case is dope! Can you post which exact syntax you want to have? Because there are various limitations while implementing this, I was thinking a lot recently about how to approach this, I want to achieve better code reusing. Btw if you manage to write this codemod it would be awesome to open-source it :D Nested css interpolations would be hard to implement, I was thinking about being able to write this

const shared = css``

const className = css`
${shared}

`

And I would try implement 'sniffing' based on the prepending dot. It would allow to use css as className or interpolate it's styles into other css or styled.

jonatansberg commented 4 years ago

Yeah, that is pretty much what I'm after as well. I understand that this is a pattern that has a lot of caveats, and that you're most of the time better of solving this in other ways. Unfortunately since this used to work with Emotion, we need to find a way to either codemod it away or make it work with Linaria...

Here is a complete example. Code like this:

const shared = css` 
  color: peachpuff;
`;

const otherStyles = css`
  ${shared}
  background: crimson;
`

const InlineComponent = styled.span`
  text-transform: uppercase;
`

const Component = styled.div`
  padding: 1rem;
  ${otherStyles}

  ${InlineComponent} {
    font-weight: bold;
  }
`

Should preferably turn in to something thats the equivalent to this:

.InlineComponent {
  text-transform: uppercase;
}
.Component {
  padding: 1rem;
  color: peachpuff;
  background: crimson;
}
.Component .InlineComponent {
  font-weight: bold;
}

This works just fine when you remove the css tags and just use string literals, but thats really hard to do in an automated way since the css tags could be used either as a class name or as a style string...

jayu commented 4 years ago

Good, I will try to work on some PoC next week

jayu commented 4 years ago

After trying to implement composition I understood that it is not possible in a way I thought it would be. My approach was to grab composed class styles and just inline them inside another class. But if we consider a situation that shared styles are imported from other files, and what's worse, it imports another file, we end up with a need to resolve all styles during preevaluation, which will kill the performance at all and would be very hard to implement.

So the approach we can take is similar to what CSS-Modules do with composes: property. We can compose css classes that have some limitations. It could work like described here https://webpack.js.org/loaders/css-loader/#composing

Since we have merge class names, rules from composed class names will override corresponding rules in base class regardless of the ordering, so

const shared = css`
   color: red;
`
const base = css`
   ${shared}
   color: blue;
`

will work the same as

const shared = css`
   color: red;
`
const base = css`
   color: blue;
   ${shared}
`

Which will result in blue color due to overridden value in CSS I need to check how exactly composes works in CSS modules, but In each example that I found it is at the beginning of the declaration, so I guess we have to do the same. Eventually, we can distinguish between interpolation of shared at the begging and at the end of css definition, but it brings additional complexity .

I guess this not match the behavior from emotion since the code above would act differently in Linaria Overriding the same prop is an egde case, but may break a lot of styling, unfortunately. How it will work with you use case @jonatansberg?

It doesn't matter wheter we use composes or extends keyword or just plain interpolation, so we can make syntax like in emotion or like in css-modules or even both. But having composes keyword explicitly makes it easier to reason about in terms of how it works.

Update : We have to have composes: keyword explicitly because, without it, we cannot distinguish between object/string being interpolated vs class name during the pre-evaluation.

Nested interpolation would no be allowed so it will not work

const base = css`
   color: blue;
   .nested {
       ${shared}
   }
`
jonatansberg commented 4 years ago

@Jayphen: Does this help in any way?

Jayphen commented 4 years ago

The behaviour of overriding properties regardless of the interpolation order is fine — although it's not exactly equivalent to the spec CSS behaviour, it's the intended outcome when "mixing in" styles this way (at least in the examples I've seen in our projects).

In our case, the interpolation isn't necessarily always at the beginning of the declaration, so if that is a limitation then the codemod would need to 'hoist' the interpolation(s?).

I believe we also have cases where interpolations are nested, and there's no way to codemod that, which will still be an issue.

edit: I see you updated your post @jayu — what would the syntax look like with the composes keyword?

jayu commented 4 years ago

@Jayphen So with my current approach it could look like this :

const shared = css` 
  color: peachpuff;
`;

const otherStyles = css`
  composes: ${shared}
  background: crimson;
`

const InlineComponent = styled.span`
  text-transform: uppercase;
`

const Component = styled.div`
  padding: 1rem;
  composes: ${otherStyles}

  ${InlineComponent} {
    font-weight: bold;
  }
`

And effectively it would act like composes is hoisted to the top of declaration, so styles from particular tag will override the composed styles because of the class names composition. So you can put composes: anywhere in the tag, but styles from the tag will have priority

jayu commented 4 years ago

I think I found a way to go with my initial approach to spread the styles... I will test the performance of both solutions.

jayu commented 4 years ago

I opened a draft PR with PoC implementation, the allowed syntax is as below, please find the description in the PR #615

const fragment = css`
  color: red;
  font-size: 40px;
  padding-top: ${(props) => props.height || '200px'}; 
`

const anotherFragment = css`
  color: yellow;
  ${fragment};
  border: 10px solid black;
`

const component styled.h1`
color: black;
${anotherFragment}
.sub-class {
    ${fragment}
}
`
kondei commented 3 years ago

Are there any updates? I want a composition function. I'm reluctant to use styled-components because linaria doesn't have it. Without CSS-in-JS side composition for deterministic behavior, We have to fight against non-deterministic behavior of CSS classes that depend on the uncontrollable order in which the bundler outputs them.

jayu commented 3 years ago

I wish I could have more time to tackle that, but unfortunately, I don't.

Version with overloaded css to be used to generate class name or fragment was rejected and It was a good decision at the end.

The main issue in using any other tag for fragment is syntax highlighting inside the template string, which I believe is not a big blocker for implementing that feature. I did not investigate, but the available tools should support syntax highlighting for any language syntax in that or another way, so we check that first and then agree on the tag name.

Besides the main issue, @kondei if you have a problem with the non-deterministic order of CSS classes in your bundle, you should try using some lint rule to sort your imports, which would guarantee that imports are sorted based on deterministic rules. That might be painful at the beginning, as many of the styles can break, but there is no other way to tell the bundler in which order it should output the classes into the CSS bundle. For mini-css-extract-plugin there is a famous issue of conflicting-order https://github.com/webpack-contrib/mini-css-extract-plugin/issues/250. Many people suggest to just ignore the warnings, but in fact, this problem is quite significant for bigger projects with a lot of code, since with non-deterministic order of imports, you can for example swap imports in one component, which would swap the order of CSS output somewhere higher in the dependency tree, in a place in the app, where you would never expect that to happen. It's really hard to track and predict those bugs. Sorting imports is the only way to solve that. Even with fragment tag, you will encounter that problem sooner or later

brvnonascimento commented 2 years ago

Couldn't we have a suffix on the variable name that would prevent its transformation into a className? For example:

const heading1__fragment = css`
  background: red;
`
dev-m1-macbook commented 2 years ago

@brvnonascimento or maybe we can have another tag for that:

const highlightedBackground = cssFragment`
  background: yellow;
`

though I'm wondering whether webpack will tree-shake this without extra work from linaria / etc

IgnusG commented 2 years ago

@brvnonascimento or maybe we can have another tag for that:

const highlightedBackground = cssFragment`
  background: yellow;
`

Looks like we almost went full circle to the original issue's suggestion 😅

glomotion commented 2 years ago

So I have just landed here, after trying to evaluate Linaria as a potential style tooling base for a new DS i'm working on. My team and I had dearly hoped to engineer a props-based style approach much like https://styled-system.com/api. Sadly though, we'd need interpolation of the css fragment within a style block (so that we can axs component props).

Its now become clear, that such an aspiration is not currently possible within a Linaria based project (or am I wrong there?)

If its not possible currently, are there any plans in the roadmap to address this gap between styled-component's API and Linaria's?

shiro commented 1 year ago

@glomotion I don't know whether or not the core library will add it in the (near) future, but if you just want to use it today, the following code will just work:

export const style = (s: TemplateStringsArray, ...expr: Array<string | number | CSSProperties>): string => {
    let res = "";
    for (let i = 0; i < Math.max(s.length, expr.length); ++i) {
        res += s[i] ?? "";
        res += expr[i] ?? "";
    }

    return res;
};

// usage
const shared = style`
  border: 1px solid blue;
`;
const someClass = css`
  background: red;
  ${shared}
`

It just turns the contents into a string and gives you IDE completion (at least in IntelliJ due to the word style being handled by the styled component plugin).
Is it a hack? yes...
I've been using it on a pretty huge codebase for 2 years now without issues though.

mohammedhammoud commented 5 months ago

Any plans do support this?

Firsh commented 2 months ago

Since I don't actually use css from linaria in its current form, but the syntax highligter extension in VSCode relies on that syntax I use this for static stuff:

export const css = (strings: TemplateStringsArray): string => strings.join("");

For anything dynamic, I use CSS vars or don't put them inside css`` at all. This is like a mixin to avoid repeating myself when simple inheritance is not enough and want to compose something.