kentcdodds / jest-glamor-react

Jest utilities for Glamor and React
https://npm.im/jest-glamor-react
MIT License
98 stars 24 forks source link

Sort CSS output? #3

Closed kentcdodds closed 7 years ago

kentcdodds commented 7 years ago

Otherwise, it appears that it's kinda random and the diffs are really weird. If they could be sorted then maybe they'd stay in the same place? It's unclear. I think that this could help with that though...

kentcdodds commented 7 years ago

I think that this is no longer an issue.

chrisnojima commented 6 years ago

@kentcdodds would it be possible to reopen this? I just integrated storyshots/storybook jest testing and I use this utility so my glamor css names are 'stable' between CI runs (awesome) but sadly I had to turn it off because I get diffs where the order of the .glamor-* changes

example:

 - .glamor-3,
    - [data-glamor-3] {
    -   font-size: 13px;
    -   line-height: 17px;
    -   color: rgba(0, 0, 0, 0.75);
    -   -webkit-font-smoothing: antialiased;
    -   text-rendering: optimizeLegibility;
    -   font-family: OpenSans;
    -   font-weight: 600;
    - }
    - 
      .glamor-4,
      [data-glamor-4] {
        font-size: 13px;
        line-height: 17px;
        color: rgba(0, 0, 0, 0.75);
    @@ -58,10 +47,21 @@
        -webkit-flex-direction: row;
        -webkit-box-lines: multiple;
        -webkit-flex-wrap: wrap;
      }

    + .glamor-3,
    + [data-glamor-3] {
    +   font-size: 13px;
    +   line-height: 17px;
    +   color: rgba(0, 0, 0, 0.75);
    +   -webkit-font-smoothing: antialiased;
    +   text-rendering: optimizeLegibility;
    +   font-family: OpenSans;
    +   font-weight: 600;
    + }
    + 

which causes my snapshot to fail

kentcdodds commented 6 years ago

Huh, that's odd. I've not had any trouble with things for several months. It's surprising to me that you'd see this. I'm guessing that there's some sort of race condition that's leading to some styles getting registered with glamor earlier/later locally than it does on CI.

If you don't mind, could you fork this project and try sorting things and see how that works in your project? Then I'll try it out on mine and see whether this is something we should try.

kentcdodds commented 6 years ago

I'll open the issue again if we decide to do something about it :+1:

chrisnojima commented 6 years ago

hmm. So if i hand edit the file in my node_modules this works: (https://github.com/keybase/jest-glamor-react/pull/2) but trying to build it locally it fails for some reason. Anyways, thats my quick and dirty fix which does sort the output in ascending order instead of being kinda random. Not optimized but minimally invasive

kentcdodds commented 6 years ago

Hmmm... That looks fine. But I'm still not 100% certain I want to do it. Could you figure out how to make it build locally. I'd love it if you could publish something to npm (@your-username/temp-jest-glamor-react or something) and I can try that out on my project to see how it goes.

chrisnojima commented 6 years ago

ok i figured it out. I was trying to do this all inside my node_modules and installing with yarn to build the dist added some sub dependencies which freaked my project out.

so you should be able to change your package.json to use

    "jest-glamor-react": "git://github.com/keybase/jest-glamor-react#sort-styles-off-of-312",

and it should work

kentcdodds commented 6 years ago

Ok, cool. I'll give it a try when I get some time.

chrisnojima commented 6 years ago

@kentcdodds no rush, just a one-time gentle nudge :)

kentcdodds commented 6 years ago

Thanks for the nudge @chrisnojima. Sorry I haven't gotten to it yet. Now I'm on a long break. Not sure when I'll get to it. If you can have some other folks try it though and find it favorable then I'll merge it without trying it myself 👌

asvetliakov commented 6 years ago

@chrisnojima I had the same problem with emotion. The problem is that if the order of your tests is somehow different (async, running single it() rather than whole test suite while working), then the order of styles will be different too.

I fixed it by using this:

const { sheet, flush } = require("emotion");

afterEach(() => {
    flush();
});

in jest's setupTestFrameworkScriptFile may be it'll help

kentcdodds commented 6 years ago

Ah yes, I have to do that with glamor as well 👍

chrisnojima commented 6 years ago

Cool, finally got around to implementing this on our side and I think it does stabilize it. thanks!