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

Generate friendly class names #650

Closed
mike-shtil opened this issue Jun 21, 2021 · 21 comments
Closed

Generate friendly class names #650

mike-shtil opened this issue Jun 21, 2021 · 21 comments

Comments

@mike-shtil
Copy link

Is your feature request related to a problem? Please describe.
For better debugging, it's useful to quickly understand which component controls a given markup via friendly class names.

Describe the solution you'd like
Have an option to generate friendly class names in DOM:
<div class="sxrb1g9">hello</div> becomes <div class="MyReactComponent__MyStyledComponent__sxrb1g9"hello</div>.
Would work similar to the displayName option for styled components: https://styled-components.com/docs/tooling#better-debugging

Suggested syntax:

createCss({
  ...
  debugClassNames: true // or e.g. process.env.NODE_ENV !== 'production'
})

Describe alternatives you've considered
Alternative is to, in unfamiliar markup, hunt for the right component by tag names and trial and error 😒

Additional context
This feature exists in CSS modules, styled components and many other solutions, would dare calling it a standard.
I believe this part will need to be expanded a bit to accept new configs (and also get component names somehow)
https://github.com/modulz/stitches/blob/676eac2c37f50ec1598c4daba3cfcb35dca92287/packages/core/src/features/css.js#L81

@peduarte
Copy link
Contributor

This isn't something that we can offer out-of-the-box. In order to generate class names based on your variable names, it needs to happen at build time - or conveniently via babel.

There is a babel package that does this for Stitches, but I haven't personally tried: https://www.npmjs.com/package/babel-plugin-transform-stitches-display-name

@mike-shtil
Copy link
Author

Yeah, I see the logic in what you're saying. Appreciate the link!

@LucasUnplugged
Copy link
Contributor

Commenting here because this is also something I'm looking for. While the babel package above is great when using React Dev Tools, in terms of inspecting the DOM itself, this feature request would be great!

@peduarte I'm having trouble getting the Stitches to build locally, but here is my suggested implementation for this:

Screen Shot 2021-07-22 at 1 39 35 PM

If I'm understanding everything correctly, that means one could do the following:

const Flex = styled('div', 'Flex', { display: 'flex' })

And that would result in a DOM output like this:

<div class="my-prefix-c-Flex+ktIlUM">...</div>

(Obviously the + connector could be something else, like _)

What do you think of adding something like this?

I imagine this is doable with a babel plugin, without having to add the extra string (by using the const name). The advantage there is no extra work needed in terms of DX, after setup. The disadvantages are that you can't get it out-of-the-box; you can't customize the string (it will always be the const name); and it requires more setup (minimal setup is one of the many things I like about Stitches).

@peduarte
Copy link
Contributor

Hey!

Thanks for sharing your thoughts.

If I'm understanding everything correctly, that means one could do the following:

const Flex = styled('div', 'Flex', { display: 'flex' })

Yes, and no. It will compile, but it's invalid. It's also a side-effect of something else. We're still polishing this API and I have a call with @jonathantneal next week to talk about this.


If there's enough demand for passing through a custom class name, it's something we could consider adding to Stitches.

I totally agree with the minimal set up, but I think for now, babel is the way to go for this. Babel can also be used to add class name as well as displayName. Maybe @afzalsayed96 can shed some light on this?

@afzalsayed96
Copy link
Contributor

I'm not quite able to figure out how this would be possible via Babel plugin. Unless we are talking about transforming library code itself which would be too coupled with the implementation.

@afzalsayed96
Copy link
Contributor

Update: Found a way to do this manually in userland:
https://codesandbox.io/s/angry-dew-mn12z?file=/src/App.tsx

Next step: Automate this via Babel plugin

@LucasUnplugged
Copy link
Contributor

Update: Found a way to do this manually in userland:
https://codesandbox.io/s/angry-dew-mn12z?file=/src/App.tsx

Next step: Automate this via Babel plugin

Awesome job, @afzalsayed96! One question, though: doesn't using a separate call to createCss create a separate theme output each time? So if we wanted to create a dark theme, for instance... I'm not even sure how we would do that with this implementation, but I think it would involve a whole bucket-load of theme() calls.

I was thinking that what could be done instead is to decorate the styled function — something like this.

However, it looks like there is more that needs to be decorated than what I've done there — not to mention I'm seeing a TS error, which I'm not understanding the root cause of:

JSX element type 'Main' does not have any construct or call signatures.ts(2604)

I don't understand how returning the output of the original styled function from a decorated styled function could result in a different signature, but I'm clearly missing something.

@LucasUnplugged
Copy link
Contributor

LucasUnplugged commented Jul 24, 2021

@afzalsayed96 I think I figured it out, by leveraging your Object.assign(...) approach, from the babel plugin. Here are two different approaches to implementing it — I'm not sure which would be easier to do with babel:

  1. By decorating styled, and changing the call signature to have a name string:
const Text = styled('Text', 'p', { fontFamily: '$system' });
  1. By creating an extra function (withClass), and wrapping that around each call to the original styled:
const Box = withClass('Box', styled('div', {}));

Here are the 2 functions:

// Decorated `styled`
export const styled = (
  name: string,
  target: React.ElementType,
  styles: CSS
) => {
  const Original = stitches.styled(target, styles);
  const className = Original.className.replace('c-', `${name}-`);
  return Object.assign({}, Original, {
    defaultProps: { className },
    displayName: name,
    className,
    selector: `.${className}`,
    toString: () => className,
  });
};

// Wrapper function
export const withClass = (name: string, Original: any) => {
  const className = Original.className.replace('c-', `${name}-`);
  return Object.assign({}, Original, {
    defaultProps: { className },
    displayName: name,
    className,
    selector: `.${className}`,
    toString: () => className,
  });
};

Both are resulting in rendered HTML such as this, which I think is pretty good:

<main class="demo-c-PJLV demo-c-PJLV-idwRXDY-css demo-Box-PJLV">

You guys would have a better sense of whether my string replacement is all correct (and if I missed anything), so please tweak that as needed.

LucasUnplugged added a commit to LucasUnplugged/stitches that referenced this issue Jul 27, 2021
@LucasUnplugged
Copy link
Contributor

LucasUnplugged commented Jul 27, 2021

So I was able to get the code base to build (Node version issue), and what I'd like to propose on this front, is what you see in this commit to my fork:

  1. The styled function would an optional 3rd param (string), which could be used to generate friendly names.
  2. If the 3rd param is passed, it is used both for the class names, AND for the displayName property.
  3. This could remain a sort of "hidden feature", if you'd like to maintain the current public API, but at least the transform-stitches-display-name babel plugin could leverage it to provide more meaningful display names AND class names.

Usage (possibly done through the plugin) would be as simple as:

const Box = styled("div", { boxSizing: "border-box" }, "Box")

I've tested this with a local build, and it worked beautifully, with an output such as:

<header class="c-Box-lesPJm c-Flex-dhzjXW c-Row-lfylVv c-Header-fVzExb"><h1>Stitches Demo</h1></header>

<button class="c-Box-lesPJm c-Button-bItMgZ c-Button-bItMgZ-liHaWo-primary-true">Primary</button>

And a React Dev Tools output of:

Screen Shot 2021-07-26 at 9 03 03 PM

That looks pretty sweet to me, and would make understanding where each style is coming from (via the class names) very easy.

@peduarte
Copy link
Contributor

@LucasUnplugged this is awesome. Thank you for your effort and exploration.

I think it would be a shame to have these comments hidden under a closed issue. But right now, this is out of scope. However, I'm 100% open to considering this feature, and get feedback from other users around the API.

I think the best way to go forward is to open up a discussion. What do you think?

@LucasUnplugged
Copy link
Contributor

I think the best way to go forward is to open up a discussion. What do you think?

Sounds good! I'll open a discussion, and reference this conversation there.

LucasUnplugged added a commit to LucasUnplugged/stitches that referenced this issue Aug 8, 2021
LucasUnplugged added a commit to LucasUnplugged/stitches that referenced this issue Aug 8, 2021
@giladv
Copy link

giladv commented Sep 25, 2021

so for those coming from css modules and would like to still work in a similar fashion, i thought about a simple-stupid (and maybe even naive) solution for friendly classNames. it might make no sense but:

import { css as nativeCss } from '@streamelements/frontend-ui';

export function css(namespace, styles): any {
  const ReadableCssFuncs = {};
  Object.keys(styles).forEach((styleKey) => {
    const cssFunc = nativeCss(styles[styleKey]);
    ReadableCssFuncs[styleKey] = (...args) => {
      return `${namespace}-${styleKey} ${cssFunc(...args).className}`;
    };
  });
  return ReadableCssFuncs;
}

now you can create a style.ts for each component (like you would create a style.css)
and use it like so

export default css('Sidebar', {
  root: {
    backgroundColor: 'blue',
  },
  menuItem: {
    backgroundColor: 'green',
  }
});

then use it in your jsx like you would with css modules

import s from './style.ts'

<div className={s.menuItem()}>...</div>

now it will render something like

<div class="Sidebar-menuItem se-ds-c-fNnBJi">...</div>

@s-h-a-d-o-w
Copy link

@LucasUnplugged How can your improvements be used? As of v1.2.6, stitches at least doesn’t seem to do anything automatically.

@LucasUnplugged
Copy link
Contributor

@s-h-a-d-o-w You can install it like so, in your package.json dependencies:

"@stitches/react": "https://gitpkg.now.sh/withneutron/stitches/packages/react?release/650_Friendly_classnames",

I've re-applied my fix to v1.2.6, with some improvements. It should also work with this Babel plugin I had made, but I haven't tested it yet.

Note: without the Babel plugin, you need to add a string as the last param you're passing into your styled function, to get friendlier class names:

// Will output a className such as c-Button-PJLV
const Button = styled('div', {}, 'Button')

@s-h-a-d-o-w
Copy link

Oh! I totally missed that your work wasn't in a PR but a fork!

That's a shame that it's not supported officially, seeing how all other major CSS in JS tools offer ways of generating readable class names.

Still, thanks for the explanation!

@LucasUnplugged
Copy link
Contributor

@s-h-a-d-o-w I can try doing a PR again, but I know that at the time, the Stitches team wanted to focus on getting the v1 API stable; I'm not sure if their stance may have changed since then.

That said, I'm using Stitches actively for my own startup, so may consider doing proper (npm) releases of my fork, if this limitation doesn't get addressed in some way, in the future. Not sure if that would address your concern (we're a tiny team right now).

@s-h-a-d-o-w
Copy link

Thanks for letting me know but yeah, since forks have a tendency to stop receiving updates pretty quickly, I'm not keen on trying to sell that to my team. 😄

@LucasUnplugged
Copy link
Contributor

@s-h-a-d-o-w I've opened an official PR to try to get this in, at #916. Please feel free to chime in there. I've been using that build for 2 weeks now, without issue, and find this a huge improvement for debugging purposes.

@bitabs
Copy link

bitabs commented Feb 8, 2022

Please ship this PR as we struggle a lot, unfortunately. Thanks 🙏

@bartcheers
Copy link

Closing the loop here: withConfig has been introduced after this issue was closed, achieving the desired behavior. It's still missing in the docs, but instructions can be found here.

@s-h-a-d-o-w
Copy link

@bartcheers
It doesn't and this issue should be reopened.
Please note that giving each component a custom name manually is not what was requested here and is not an acceptable workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants