stitchesjs / stitches

[Not Actively Maintained] CSS-in-JS with near-zero runtime, SSR, multi-variant support, and a best-in-class developer experience.
https://stitches.dev
MIT License
7.74k stars 253 forks source link

IE11 support without Proxy, alternative API, and feedback #27

Closed eddyw closed 3 years ago

eddyw commented 4 years ago

Hey there,

I was checking the code and it uses Proxy which doesn't have IE11 support. I initially thought that maybe I could use a Proxy polyfill. However, this kind of polyfills require to know all properties at the time the Proxy is created which isn't possible here.

Would you consider also an alternative (additional) API? Something more close to CSS-in-JS syntax? For example:

const { createCss } = require("@stitches/css");

const css = createCss({
  screens: {
    desktop: (rule) => `@media (min-width: 700px) { ${rule} }`,
  },
});

const button = css.compose(
  css.color("gray"),
  css.padding("1rem"),
  css.color("black", ":hover"),
  css.desktop.backgroundColor("tomato", ":hover"),
  css.borderColor("red"),
  css.desktop.borderColor("black")
);

const alternative = {
  color: "gray",
  padding: "1rem",
  "&:hover": {
    color: "black",
    backgroundColor: { desktop: "tomato" },
  },
  borderColor: { default: "red", desktop: "black" },
};

Here button and alternative are equivalent. It's closer to CSS and it doesn't really require a Proxy to know which are utility or css props, for instance. It's also fairly easy to type in TS.

A working example of the above:

const toStitchesCSS = (obj, pseudo) => {
  const call = (cssIns, prop, value) => {
    return cssIns[prop](value, pseudo);
  };
  const keys = Object.keys(obj);
  const comp = [];

  for (const key of keys) {
    const value = obj[key];
    if (typeof value !== "object") comp.push(call(css, key, value));
    else if (key.startsWith("&:"))
      comp.push(toStitchesCSS(value, key.replace(/^&/, "")));
    else
      comp.push(
        ...Object.keys(value).map((screen) => {
          if (screen === "default") return call(css, key, value[screen]);
          return call(css[screen], key, value[screen]);
        })
      );
  }

  return css.compose.apply(css, comp);
};

Then you can:

const sortAsc = classes => classes.split(' ').sort().join(' ') // Just to compare
const buttonClasses = sortAsc(button.toString())
const alternativeClasses = sortAsc(toStitchesCSS(alternative).toString())

console.log({
  button: buttonClasses,
  alternative: alternativeClasses,
  isEqual: alternativeClasses === buttonClasses // true
});

Of course, this is an abstraction on top of the current implementation that uses Proxy but it's just to demonstrate that it could also work with an alternative (additional) API closer to CSS-in-JS object syntax (and it'd work in IE11 πŸ™ˆ)

christianalfoni commented 4 years ago

Hi @eddyw and thanks for the feedback πŸ˜„

The reason sttiches has this API is

  1. Typing

  2. Utilities

  3. Performance

  4. The object syntax sucks in terms of intellisense and typing. The reason is that:

{
 ":hover": {
    color: 'red'
  }
}

Now we have allowed any kind of key on the object (Cause we allow any string being pseudo selectors), meaning you will not get an error if you type incorrectly. Also when hitting : to insert a value the intellisense does not trigger and shows you the optional values, you have to explicitly open it. So:

// This is typed with documentation
css.color

// Instantly gives you suggestions on arguments
css.color()

css.color('red', ':hover')
  1. Utilities are not restricted to a single value, you can create all sorts of utilities. So for example:
css.boxShadow(0, 0, 0, 5, 'RED_500', ':hover')

Here we create a box shadow utility with multiple arguments where each argument can be typed as well. You would not be able to express this in object style. You could argue using [0, 0, 0, 5, 'RED_500'], but it would again be a reduced typing experience... if even possible to type.

  1. We do not have to "parse" the object to figure out what you want, every call to css creates the resulting "atom", as we call it

But yeah... I totally get that you feel most comfortable with {} syntax, I did too. But the functional approach gives additional benefits πŸ˜„

About IE11. Only way to make that work with current API would be to create a library which actually exposes all the CSS properties, which would increase the payload quite a bit.

On a more principle point of view I really do not want to support IE11, cause it is just insane that we still support it. Especially with the new Edge that has IE11 mode integrated in its tabs (Allowing to run legacy apps and new apps in the same browser). As developers we have to push back on support IE11 cause it has an insane cost for customers and drives developers insane.

Like... we could expose an object based API, but there are so many CSS IN JS solutions out there, that also supports IE11. So really just want to say.... it is out of scope for this library πŸ˜‰

eddyw commented 4 years ago

@christianalfoni I'd have to argue a bit about 1. typings since we have something similar:

const someCss: CSS = {
  color: 'textGrey', // << Has intellisense & auto-complete
  bgColor: {
     xs: 'blockWhite', // same here
  },
  cssPseudo: { // << yeah, that's how we know it's pseudo actually haha
     '&:hover': {
        ...
     }
  },
  boxShadow: CSS.shadow(....), // << we have this
}

CSS is actually the utils object with typed methods but it's exported as:

export const CSS = { ... }
export type CSS = CSSProps // < not exactly but just to show the point

Anyways, I do agree that using that functional approach is actually faster in terms of performance (because there is no need to "parse" the object) .. and yeah I also agree with everything else πŸ˜…

Sadly we can't always choose not to support IE11 (as developers) until it officially dies in 5 years & 5 months from now πŸ™ˆ

Before closing this thread, do you have somewhere a .. roadmap, timeline, or a description of the scope of this library?, like where it is going, what's left to-do πŸ˜… for people interested in the project who may want to contribute.

christianalfoni commented 4 years ago
  1. Ah, yeah, I did not mean you do not have typing... it is just the intellisense experience is not as good :) With the functional approach the intellisense automatically pops up as you type, unlike object syntax where you have to explicitly ask for it either by hovering the property or hit "ctrl + ." on the value :-)

I updated the main monorepo README with a roadmap. We have really reached the planned scope of the library now, though with feedback of IE11 support and also React Native support, things might change when we get the core to the point where we want it to be πŸ˜„

I am curious... what are the arguments on IE11? Why can they not use the new Edge browser? It has an actual IE11 environment included.

eddyw commented 4 years ago

@christianalfoni yeah, that's true regarding typings.

In my case, we're porting a website from WP to NextJS + headless CMS, so the requirement is that it must work as it previously did in WP which means IE11 support.

Not to open another issue (I'll rename the title), there is an issue with specificity with pseudo selectors. Consider this code:

const className = css.compose(
  css.color("red", ":hover"),
  css.color("green", ":focus")
);

(Pretend the className is applied to a button) what you'll see is:

However, if you change the order:

const className = css.compose(
  css.color("green", ":focus"),
  css.color("red", ":hover")
);

Tailwind and other Atomic CSS-in-JS libs solve it similarly: https://tailwindcss.com/docs/configuring-variants/#ordering-variants Basically, styles should always be inserted in a deterministic order (e.g: :hover first, then :focus, etc). However, Stitches isn't deterministic in this case. An idea how to easily increase the specificity is to just duplicate the class name:

.di_c_1.di_c_1.di_c_1:focus { ... }
.di_c_0.di_c_0:hover { ... }

This way you don't need to worry about sorting it. However, it'd still need to check what kind of pseudo selector is used and how many times to duplicate it. Ref, I saw it here first πŸ™ˆ : https://twitter.com/kripod97/status/1259238906802962433

Hm, I also found out that the same issue happens with shorthand properties:

const className1 = css.compose(
  css.border("1px solid green"),
  css.borderWidth("5px")
);

In this case, the element would have a border width of 5px. However, if the first time the styles are defined in different order:

const className1 = css.compose(
  css.borderWidth("5px"),
  css.border("1px solid green"),
);

Then, the element's border width will be 1px.

peduarte commented 4 years ago

Since this issue was created, the API has been changed to use the object syntax.

We still use Proxy, which is not supported by IE11. Given this https://dev.to/swyx/ie11-mainstream-end-of-life-in-oct-2020-3gdj Im not sure whether we should support it or not.

Thoughts?

eddyw commented 4 years ago

@peduarte wow, that's like the best thing I read this year (the article)

Thoughts?

My personal opinion is that maybe it isn't worth the time to invest in IE11 support. Besides, if stitches now support object syntax, then this could work -> https://github.com/GoogleChrome/proxy-polyfill since the object keys (in object syntax) are known πŸ€”

kuldeepkeshwar commented 4 years ago

@peduarte @christianalfoni we can get rid of proxy & use a babel plugin/macro to transform styled.<tag>'...' to styled(<tag>)'...'.

I would suggest going for macro as it works seamlessly with react ecosystem(CRA/Nextjs ..etc)

I have followed similar approach for filbert-js.

Happy to open up a pull request

peduarte commented 4 years ago

@kuldeepkeshwar hey, thanks for chipping in! πŸ™

@hadihallak @christianalfoni what do you guys think?

For me, the most important thing is to keep the barrier to entry low. We're not planning on supporting IE11, so keeping the proxy is not an issue.

Curious to learn more about the pros and cons.

@kuldeepkeshwar if you'd like to create a PR to better demonstrate the idea, that would be cool too.

kuldeepkeshwar commented 4 years ago

@peduarte here you go https://github.com/kuldeepkeshwar/example-stitches-macro ps still we need to cover up on edge cases e.g transformation get applied only to styled from stitches package

Source code:

Screenshot 2020-09-02 at 3 35 59 PM

Compiled :

Screenshot 2020-09-02 at 3 35 29 PM
kuldeepkeshwar commented 4 years ago

@peduarte @hadihallak @christianalfoni right now I can think of πŸ‘‡

Pros:

Cons:

peduarte commented 4 years ago

@kuldeepkeshwar Can this solve the current issue with browser prefixes in SSR? https://stitches.dev/docs/server-side-rendering

kuldeepkeshwar commented 4 years ago

@peduarte sorry didn't get it, how would prefixing solve for Proxy support?

peduarte commented 4 years ago

@kuldeepkeshwar

we can do multiple stuff with babel-plugin/macro

thought maybe we could do something to solve that

kuldeepkeshwar commented 4 years ago

@kuldeepkeshwar

we can do multiple stuff with babel-plugin/macro

thought maybe we could do something to solve that

Ok here we are talking about couple of things

jonathantneal commented 3 years ago

In our current documentation and tests, I exclusively see things like styled(TagString | Component, …). I’d be interested in dropping the Proxy (and styled.TagString) as an anti-pattern; not necessarily because of IE11. Are there strong feelings to the contrary?

It looks like csstype already comes into this project by way of @types/react. I’d be interested in bringing it in formally and exclusively for property completion. As of TypeScript 3.1, it seems performance issues were mitigated. Dead Code Elimination would largely be a non-issue since the typings would be exclusively shipped to IDE experiences.

LukasBombach commented 3 years ago

Hey there, here are my uncalled for two cents. My team and I have been working on and implemented the homepages Germany's largest newspapers (Welt & Bild) and we are currently working on another major one. We would love to use Stitches for this, everyone in our team is super excited by its features and API.

The thing with IE11 is that it is not up to us to decide whether we want to support it or not. We have roundabout 3% traffic coming from IE11 and our top level management would not want to lose that profit (which is a lot). For us developers it seems it is hard to win that fight against our company, even when arguing about development costs. We seem to have to do this.

That being said, this will probably seem very different in, let's say one or two years, when IE11 is finally done for good. And for the general use from the pov of this library I can 100% understand not to put in the effort and probably decrease code quality for a use case that still seems somewhat niche. We will probably end our support for IE11 at some point ourselves.

I am very excited about the idea to work around this problem with babel. For us, I think it would absolutely be sufficient to know there is a way to babel ourself around these IE11 constraints "from the outside" of Stitches, but our concern would be that this patch might work for now, but might break with the next release of stitches and could not be repaired. If we knew there is a way to patch this, even though it is not a core feature, but we could rely on that, that would be πŸ’―

peduarte commented 3 years ago

@LukasBombach noted. I'll report back when we have more details about the future of stitches and browser support.

Appreciate the comment!

jonathantneal commented 3 years ago

I think the biggest hurdle for IE11 is the lack of Symbol compatibility, and the lack of support from MS and other tooling libraries.

What I think we can do for this next version of Stitches is at least verify whether or not it has IE11 compatibility, and then, if it cannot be patched by Babel, identify what it would take to reach compatibility, before the release.

peduarte commented 3 years ago

Closing this since it's gone quiet and we are not supporting IE11. Thanks!

LukasBombach commented 3 years ago

That's cool from our side as wellβ€”and understandableβ€”, thank you for the discussion!

eddyw commented 3 years ago

IE11 is dead πŸ˜… So there is no point on keeping this issue open anyways: https://death-to-ie11.com/ Looking forward to the new stitches!