astroturfcss / astroturf

Better Styling through Compiling: CSS-in-JS for those that want it all.
https://astroturfcss.github.io/astroturf/
MIT License
2.28k stars 60 forks source link

Support one class approach #373

Closed user753 closed 1 year ago

user753 commented 5 years ago

I would like to use astroturf in such way

const titleClass = cls`
  color: red;
  background-color: green;
`;
<h1 className={titleClass}></h1>

similar to styled but without component

taion commented 5 years ago

We discussed this in https://github.com/4Catalyzer/astroturf/pull/348#issuecomment-520862383 as well. The issue with using a different template tag is that it wouldn't integrate with existing tooling.

user753 commented 5 years ago

@taion I think css should return class name in the same way as styled-components and emotion for records something like this could be used

const styles = {
   red: css`
     color: red;
   `,
  green: css`
     color: green;
  `
};
jquense commented 5 years ago

Now that we have class interpolation this is actually feasible. I so wonder if it's better overall. It might worth switching css to return a single class and add a new classSet or stylesheet tag for inline "files"

taion commented 5 years ago

How would you cross-reference those classes above with that object syntax though?

I guess you'd do something like:

const className1 = css`...`;
const className2 = css`...`;

instead of using an object?

taion commented 5 years ago

How much do you think it would break if we were to whitelist certain bindings like styles or fooStyles that would keep the old behavior?

jquense commented 5 years ago

yeah I mean i'd go with something like:

const btn = css`
  ...
`

const primary = css`
  composes: ${btn};

  ...
`

It should be easy enoguh to migrate existing usages to another tag if we want to do that. I'm just about to fork the vscode styled-components plugin anyway sooooo

jgoz commented 5 years ago

Two nice things about this kind of approach:

taion commented 5 years ago

So would it be terrible to check the variable name ending with /[sS]tyles$/ and build to a single class otherwise?

jgoz commented 5 years ago

I'm just dipping my toes into this stuff so far, but a convention-based approach like that seems like it could be confusing for newcomers.

Also, my codebase is mostly TypeScript, so I'm biased, but that would be impossible to express in a return type. It would have to be string | Record<string, string> and then users would have to cast every time to the appropriate type depending on whether they added a Styles suffix. Not ideal.

It seems like the natural path would be to create two separate tags for the two use cases. Astroturf might be an outlier because the css function in other libraries generally returns a single class (and Astroturf also does this when passing to a css prop). So the shortest path to consistency would be to make css always return a single class and then add a new tag (styles?) for a stylesheet fragment. That's a breaking change, but the library is still in 0.x, so a little API instability is implied IMO.

jquense commented 5 years ago

I do think i want to do that, and see what happens. Lets try it behind a config option

user753 commented 4 years ago

@jquense any news about this feature?

dkamyshov commented 4 years ago

@user753 You might be interested in my package: https://www.npmjs.com/package/ts-astroturf-tools

It does exactly that and some more stuff. It is in early beta, though.

user753 commented 4 years ago

@dkamyshov Thanks, but I use typescript with babel. Is there any plan to support direct mode through babel?

dkamyshov commented 4 years ago

@user753 There is now, here is an issue I created to track progress https://github.com/dkamyshov/ts-astroturf-tools/issues/10

mikestopcontinues commented 4 years ago

I think it's possible to overload the current css method for this. Let the .toString() method of the export object return the value of the overarching class. So you can do:

const headerCls = css`
  position: fixed;

  &.active {
    top: 0;
  }

  & .title {
    font-weight: bold;
  }
`;

This would allow you to use it in all the same ways you use the other exported classes, and be totally backwards compatible.

lostfictions commented 3 years ago

Just wanted to poke in and ask if there's any possibility of something like this landing in v1?

@jgoz correctly points out that the one-class behaviour is much nicer for static analysis (unused classes just become unused variables, so a linter can trivially warn about them) and much better for TypeScript and Flow users (no possibility of misspelling a class name and getting a runtime error). As mentioned, Astroturf is something of an outlier in css-in-js libs due to its current behaviour -- most other libraries follow the one-class-per-declaration approach for the css tag -- so this would also bring its behaviour more in line with what most folks expect.

This has actually been a sticking point with some coworkers when I've suggested using Astroturf, and because of the abovementioned desirable properties folks have ended up going with Linaria. I think it's a shame since in my experience Astroturf has always been so much easier to set up and less error-prone, but I can't say I disagree with their conclusions.

jquense commented 3 years ago

already in v1 👍

mikestopcontinues commented 3 years ago

Awesome! In the newest README, it looks like you can do:

const a = css`color: green;` // a = green className
const b = css`.blue {color: blue;}` // b.blue = blue className

Is it just the docs are half-updated? Is stylesheet obsolete? I prefer the overload, as I'm hoping we can get something like:

const c = css`
  color: green;
  .blue {color: blue`}
` // c = green className, c.blue = blue className

It would also bring css in line with what you can do with styled. And for what it's worth, css in styled-components does handle exporting both, though they admittedly only use it for composition.

jquense commented 3 years ago

The readme isn't up to date, try https://astroturf.netlify.app/getting-started/

dreyks commented 3 years ago

hey! is it intended that css produces a string consisting of 2 classnames? it's not a big deal for react but doesn't work out of the box with vanilla classList.add

alexamy commented 1 year ago

Took me a good amount of time to find out why class name from css isnt working with classList.add. I find this behaviour confusing.