Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate stiches.js to replace emotion and theme-ui #2153

Closed
sebald opened this issue Jun 7, 2022 · 11 comments
Closed

Evaluate stiches.js to replace emotion and theme-ui #2153

sebald opened this issue Jun 7, 2022 · 11 comments
Assignees
Labels
status:ready Ready for development

Comments

@sebald
Copy link
Member

sebald commented Jun 7, 2022

Library to evaluate: https://stitches.dev/

Reasons:

  • way better DX
  • solves a lot of issues we have with theme-ui (we don't use much of its features, no "in place" vars that would be nice for something like border)
  • reduces our bundle size since we don't use css strings that emotion supports

Cons:

  • requires some setup regarding SSR
  • we would need to replace pseudo selectors when creating the theme, which actually may be good for performance?
  • stiches assumes that there is a default theme, which we don't really have (except some in __baseCSS), we might still make use of useComponentStylse

API:

We might use it like this:

import { styled } from '@stitches/react';
import { useComponentStyles } from '@marigold/system';

const StyledButton = styled('button', {...});

export const Component = ({ variant, size, children, ...props }) => {
  const styles = useComponentStyles(
    'Button',
    { variant, size },
  );

  return <StyledButton {...props} css={styles}>{children}</StyledButton>
}

Reuse our API?

import { Box, useComponentStyles } from '@marigold/system';

export const Component = ({ variant, size, children, ...props }) => {
  const styles = useComponentStyles(
    'Button',
    { variant, size },
  );

  return <Box as="button" {...props} __baseCSS={{ appearance: 'none' }} css={styles}>{children}</Box>
}
// system/Box.tsx
import { styled } from '@stitches/react';

export const Box = ({ __baseCSS, css, children, ...props }) => {
  const Component = styled(as, merge(__baseCSS, css));
  return <Component {...props} />;
}

Is this good? We maybe need to make a "factory"?

import { mg, useComponentStyles } from '@marigold/system';

export const Component = ({ variant, size, children, ...props }) => {
  const styles = useComponentStyles(
    'Button',
    { variant, size },
  );

  return <mg.button {...props} css={styles}>{children}</mg.button>
}
// system/factory.tsx
import { styled } from '@stitches/react';

export const md = {
  button: styled('button', {
		// basically what previously was __baseCSS
	    display: 'inline-flex',
	    alignItems: 'center',
	    justifyContent: 'center',
	    gap: '0.5ch',
	    cursor: disabled ? 'not-allowed' : 'pointer',
	    width: fullWidth ? '100%' : undefined,
	    '&:focus': {
	      outline: 0,
	    },
	  }),
  div: styled('div')
}
@sebald sebald mentioned this issue Jun 7, 2022
8 tasks
@sebald sebald changed the title evaluate stiches to replace emotion and theme-ui Evaluate stiches.js to replace emotion and theme-ui Jun 7, 2022
@sarahgm
Copy link
Member

sarahgm commented Jun 14, 2022

Some Questions from me for understanding the issue Description

  • Why do we need to replace the pseudo selectors? Do you mean the way they are implemented? Because pseudo elements and pseudo selectors are supported? here in docs
  • The Default Theme would be the Global css settings?
  • Also does that mean we need a <StyledButton/> and a 'normal' <Button/> ?

Generel
We use emotions ThemeProvider, does that mean we don't need one if we use Stitches JS? Because it's with the createStitches covered? Also the basic/default theme is there written?

From what I understand the obvious advantages are:

  1. just one tool instead of two(emotion&theme-ui) where we even don't use the full possibilities
  2. better performance

In my opinion (as far as I get it), I think we could implement this after we made next.js work 🙂 I haven't seen much conflicts.
PS: I also think in this case we should make the tokens (e.g. color00 and color.00) consistent.

@sarahgm sarahgm closed this as completed Jun 14, 2022
@sarahgm sarahgm reopened this Jun 14, 2022
@sebald
Copy link
Member Author

sebald commented Jun 23, 2022

@sarahgm

Why do we need to replace the pseudo selectors? Do you mean the way they are implemented? Because pseudo elements and pseudo selectors are supported? here in docs

I meant how our pseudos work (e.g. :hover -> :hover, [data-hover] We would need to keep/adjust that. Not that big of deal.

The Default Theme would be the Global css settings?

Not sure what you mean.

Also does that mean we need a <StyledButton/> and a 'normal' <Button/> ?

IDK depends how we implement it, I guess.


And to the other comment. Yeah, if we switch, we should take the opportunity and make stuff more consistent.

@sebald sebald added the status:rfc New issue that requires discussion and finalization label Aug 19, 2022
@sebald
Copy link
Member Author

sebald commented Nov 14, 2022

Related: stitchesjs/stitches#1109

@sarahgm sarahgm self-assigned this Feb 6, 2023
@sarahgm sarahgm added status:ready Ready for development and removed status:rfc New issue that requires discussion and finalization labels Feb 6, 2023
@sebald
Copy link
Member Author

sebald commented Feb 6, 2023

Switch to a non-CSS-in-JS Solution?

Since there might not be good support in the CSS-in-JS libs we might to consider other alternatives

Other components libs are currently moving away from CSS-in-JS library. One of the prominent ones is NextUI. Even the maintainer of Emotion says that

Server Components were designed in a way that is at fundamental odds with CSS-in-JS. So don't expect Emotion (or other popular CSS-in-JS libs) to introduce support for them. They just don't allow us to integrate with them anyhow.
see this comment here: mui/material-ui#34905

While you can make them compatible, it might no optimal or worthwhile in the future. On the other hand we currently don't really have any Next apps in productions, except the Marigold docs. This might change in the future though.

Issue from NextUI regarding their reasons to switch: nextui-org/nextui#1035


Suggestion

Investigate Tailwind? The spiritual successor of Bootstrap has a enormous community already and might be an option that also solves a lot of our challenges.

In order to have the same feature and high compatibility with our current styling approach (variants and size), there are two options:

@sarahgm
Copy link
Member

sarahgm commented Feb 6, 2023

Reference how other Design Systems use Stitches: https://github.com/radix-ui/design-system/tree/master/components

@sarahgm
Copy link
Member

sarahgm commented Mar 16, 2023

Questions tailwind:

  • how to handle simple tokens like breakpoints in tests, because the type supports just name and components. the tokens are defined in tailwind.config.cjs how to use and test them -> have to add in config but how? (example: colors: purple10 -> I can't just import the colors.ts in tailwind config build failes -> cannot find module)
  • how to fix test in useTheme with className type definition
  • how to get the theme in the marigold provider // how to get it in storybook -> why works in tests but not in app
  • what about extending a theme? How should this be handled?
  • test in Marigold.Provider
  • Storybook -> own theme???

Note:

  • right now in useComponentStyles there are two versions of it because if remove it breaks all -
  • in preview.ts same (set default to unicorn)

Next Steps:

  • more styles in tailwind ?
  • evaluate together ?
  • ???

@sebald
Copy link
Member Author

sebald commented Mar 20, 2023

what about extending a theme? How should this be handled?

can we use tailwind presets for this?

@sarahgm
Copy link
Member

sarahgm commented Mar 20, 2023

@sebald I mean the function extendTheme as I understand Presets are something else

  • maybe it works as before. the tests are correctly

@sebald
Copy link
Member Author

sebald commented Mar 20, 2023

@sarahgm ah, I think we can not use that any longer. Extending a theme has to be done via tailwind/postcss I guess

@sarahgm
Copy link
Member

sarahgm commented Mar 20, 2023

@sebald really? but the tests worked correctly I see no problem 🤔

@sebald
Copy link
Member Author

sebald commented Mar 20, 2023

Okay, then I am clearly wrong and I have to see the implementation 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready Ready for development
Projects
Archived in project
Development

No branches or pull requests

2 participants