A-gambit / CSS-IN-JS-Benchmarks

Apache License 2.0
380 stars 58 forks source link

Test cases not completely fair #41

Open jfrej opened 6 years ago

jfrej commented 6 years ago

The emotion css-mode test is not actually using emotion for colouring of the cells, which is the heaviest part. It just uses inline styles: style={{ background: `rgba(74, 174, 53, ${x})` }} which has to be super quick.

Instead, it should be using class generation like the glamorous/glamor-css test does.

I can submit a PR to fix this. It considerably changes the results table.

jfrej commented 6 years ago

Ok, I see how I got it a bit wrong. The results table mentions inline styles, so perhaps the test case is not wrong but I consider it misleading. I don't really see the point of comparing a fully dynamic class creation (like in the emotion-simple case) with a solution that uses inline styles. At best there's a case missing for a fully dynamic solution using emotion's css() function. Same with aphrodite and cxs. I'm preparing a PR to add them.

grebenyuksv-preply commented 4 years ago

I think there's a naming problem. I also got misled by the results stating that styled-jsx-inline-styles is blazingly fast, whereas there's just 1 static <style jsx> tag inside, everything else done with plain inline styles.

What I had to check myself, however, was the case of many static <style jsx> tags, which shows that there's only a small (270 vs 320) difference between N static tags and N dynamic ones. Here's the case I coded myself:

const Cell = ({ value, children }) => (
  <div className="cell">
    {children}
    <style jsx>{`
      .cell {
        display: table-cell;
        padding: 10px;
      }
    `}</style>
    <style jsx>{`
      .cell {
        background: rgba(74, 174, 53, 30);  // here I replaced the var with a static value
      }
    `}</style>
  </div>
);

Thanks for this great repo anyway, it saved me a LOT of time + your bootstrap was so easy to extend with what I wanted.