ben-rogerson / twin.macro

๐Ÿฆนโ€โ™‚๏ธ Twin blends the magic of Tailwind with the flexibility of css-in-js (emotion, styled-components, solid-styled-components, stitches and goober) at build time.
MIT License
7.87k stars 184 forks source link

Transforms only work if applied after classes like rotate, translate #20

Closed owaiswiz closed 4 years ago

owaiswiz commented 4 years ago

tw.`rotate-90 transform` works as expected however, tw.`transform rotate-90` doesn't work.

Seems anything set by transform isn't overridden later, so since transform already sets --transform-rotate: 0, rotate-90 is not applied. Same goes for all scale,skew, translate classes. Also using rotate-90! does nothing. It all only works if the order is reversed. TailwindCSS makes all rules set by these secondary utils (rotate, skew, etc) as !important so they override.

Thank you for the amazing library btw :smile:

n00b2pr0 commented 4 years ago

I'm noticing a similar behavior with classes compiling in reverse order.

Example:

// This
css={tw`m-0 mr-4`}

// Compiles to
.component {
  margin-right: 1rem;
  margin: 0;
}

// Expected
.component {
  margin: 0;
  margin-right: 1rem;
}
ben-rogerson commented 4 years ago

Damn, those are some fairly huge bugs. I'll get them fixed up pronto. Big thanks for pointing these out!

ben-rogerson commented 4 years ago

I've put together a test codesandbox on these issues and it seems to be ordering the margin classes correctly and the transforms work correctly. To demonstrate the issue, it would be great if you could fork that repo in codesandbox and trigger the errors mentioned?

TailwindCSS makes all rules set by these secondary utils (rotate, skew, etc) as !important so they override.

By looking through the css tailwind builds, I can't see any !importants being added to the transform classes - do you have a source I can see on this?

owaiswiz commented 4 years ago

I've put together a test codesandbox on these issues and it seems to be ordering the margin classes correctly and the transforms work correctly. To demonstrate the issue, it would be great if you could fork that repo in codesandbox and trigger the errors mentioned?

I saw the test sandbox. Downloaded it as zip and ran it manually on my local machine, the same issue happens (with a fresh npm install). Tried it on a different browser too, same thing. Have you tried running it locally ? If yes and it works, then there's probably something wrong on my end.

By looking through the css tailwind builds, I can't see any !importants being added to the transform classes - do you have a source I can see on this?

Sorry about that, while creating this issue I was looking at the tailwind docs for transform utilites, inspected their html and they had it set as important so I naively assumed that (seems they have !important on each of their utilites) screenshot

ben-rogerson commented 4 years ago

So strange, it's working correctly locally:

image

I also tested on these node versions just to make sure:

n00b2pr0 commented 4 years ago

Thanks for looking into this @ben-rogerson!

The project I discovered this issue uses Emotion and Gatsby. I forked the Gatsby + Tailwind + Emotion starter and was able to replicate the issue: https://codesandbox.io/s/twin-ordering-emotion-test-7b472

image

I've dug in a little and not closer to why, but glad I could produce an example outside my project.

ben-rogerson commented 4 years ago

I've made a minimal bug reproduction here.

I've noticed different behaviour between node versions when installing the reproduction locally.

Ordering issue present

1. Download zip from codesandbox + unzip
2. nvm use 10 (node v10.18.1 + npm v6.13.4 - same as codesandbox uses)
3. yarn && yarn start

image

Ordering working correctly

1. Download zip from codesandbox + unzip
2. nvm use 11 (node v11.15.0 + npm v6.7.0)
3. yarn && yarn start 

image

Not sure what's triggering the issue within node v10.18.1 yet.

owaiswiz commented 4 years ago

Not sure what's triggering the issue within node v10.18.1 yet.

I think I figured it out. It seems node (V8 really) switched to a stable sorting algorithm with v11 ( #22754, Getting things sorted in V8 )

The problem stems from here: https://github.com/ben-rogerson/twin.macro/blob/2065f65fe2733f2b8a060b59ede43bd2f05a8ed2/src/screens.js#L27-L32

For example, if I test this on node v10.18.1, this is what I get: nodess

And this is what we get on newer node versions (which uses the updated v8) (using my browser here, dont have node v11 installed right now) correctss

A possible fix would be to use this sorting function - timsort.

ben-rogerson commented 4 years ago

I've added timsort and it's working perfectly when installed locally in node 10. But it's still not working in codesandbox? ๐Ÿคท

owaiswiz commented 4 years ago

I've added timsort and it's working perfectly when installed locally in node 10. But it's still not working in codesandbox? ๐Ÿคท working as expected for me ss

ben-rogerson commented 4 years ago

This issue is solved! ๐ŸŽ‰

We've reached closing time.

n00b2pr0 commented 4 years ago

Great! 1.0.0-alpha.8 works for me. ๐ŸŽ‰

Thanks @ben-rogerson and @owaiswiz! ๐Ÿ‘