Fix for #650: Generate friendly class names #916

LucasUnplugged · 2022-01-03T12:44:03Z

(Addresses #650)

I'm giving this one another try, with the latest version (plus my suggested fixes from PRs #893 and #900).

I've been working from a build off this branch for a couple of weeks now, with no adverse side-effects. This has resulted in this kind of rendered HTML:

<section class="c-Box-lesPJm c-Flex-dhzjXW c-Column-iTKOFX c-Box-lesPJm-cScfiu-mb-6">
...
</section>

...which at this point, my brain quickly reads as (by ignoring the hashes):

<section class="c-Box c-Flex c-Column c-Box-variant-mb-6">
...
</section>

I find that vastly more helpful, for debugging purposes, than a sea of hashed classes that mean nothing to me.

Proposed API

Users can pass in a string as the last parameter of their calls to styled, like so:

// Renders as `c-Label-sOm3h4Sh`
const Label = styled("label", {...}, "Label")

This proposed solution can be combined with a babel plugin, to make friendly class names possible with no extra effort, for those using Babel.

Related Discussion

There was a related discussion at #677, but the solution proposed there by the Stitches team (defaultProps) was removed from the v1 API.

Squashed commits: [a3e0552] [0a1cdd9]

codesandbox-ci · 2022-01-03T12:44:29Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 72d7c8c:

Sandbox	Source
Stitches CI: CRA	Configuration
Stitches CI: Next.js	Configuration

LucasUnplugged · 2022-01-24T03:41:56Z

why is this being blocked?

I'm not sure it's being blocked, so much as the Stitches team has been focused elsewhere; to my knowledge, it is not an intentional blocking.

luixo

Should we add tests for the feature?
I can take over a unit test.

LucasUnplugged · 2022-01-25T01:12:34Z

Should we add tests for the feature?

I can take over a unit test.

Oh, good point. I think I had done so already in my old commit; I'll see if I can find them and add them here. Thanks!

luixo · 2022-01-25T16:34:07Z

packages/core/src/features/css.js

-const createComposer = (/** @type {InitComposer} */ { variants: initSingularVariants, compoundVariants: initCompoundVariants, defaultVariants: initDefaultVariants, ...style }, /** @type {Config} */ config) => {
+const createComposer = (/** @type {InitComposer} */ { variants: initSingularVariants, compoundVariants: initCompoundVariants, defaultVariants: initDefaultVariants, componentName, ...style }, /** @type {Config} */ config) => {
+	/** @type {string} */
+	const hash = componentName ? `${componentName}-${toHash(style)}` : toHash(style)


I believe componentName is not a part of the hash.
It probably should be something like

const className = toTailHash(config.prefix) + ['c', componentName, toHash(style)].filter(Boolean).join('-');

If I understand you correctly (and I'm not sure I do), I think both pieces of code are doing the same thing; it's just my use of a variable name hash that is causing confusion.

My code is equivalent to doing this (note the change from hash to baseClass):

let baseClass = toHash(style) if (componentName) { baseClass = `${componentName}-${baseClass}` } const className = `${toTailDashed(config.prefix)}c-${baseClass}`

So in both your code and mine, the componentName is getting conditionally added after the prefix and c-, but before the generated hash.

But I'll change the name of the variable, to avoid confusion. Let me know if you're okay with that.

Yep, they're doing the same thing, my note was about naming.
Thanks!

LucasUnplugged · 2022-01-29T18:14:09Z

I've added some tests to both core and react packages, and changed that confusing variable name.

guipolitano · 2022-02-14T18:48:47Z

Hope this PR gets merged soon

hadihallak · 2022-02-14T19:08:03Z

Hope this PR gets merged soon

@guipolitano Sadly, i don't think it's possible to get this merged as is as the API conflicts with another stitches API which is composing n number of styles into one component

const Button = styled('button', {...styles}, {...morestyles}, anotherStyledComponent)

To be clear, We love this feature and we know how important it is for the reasons outlined in the PR but we just need to think more about the API in order to maintain the DX that everyone loves about Stitches

We will think more about this and hopefully come back with a plan/decision for the API sometime next week and discuss it with @LucasUnplugged and everyone in here.

LucasUnplugged · 2022-02-14T19:11:53Z

@guipolitano Sadly, i don't think it's possible to get this merged as is as the API conflicts with another stitches API which is composing n number of styles into one component
const Button = styled('button', {...styles}, {...morestyles}, anotherStyledComponent)

@hadihallak my code shouldn't conflict with that usage, because it only applies if there are at least 2 arguments, and the last argument is a string. In your example, only the first argument is a string, so it should still work as intended.

hadihallak · 2022-02-14T19:18:29Z

That's true but it's re-using the same API which we've so far only reserved for styling so i'm wondering if we can do it differently so that it can be more clearly documented and easily readable for someone who isn't familiar with Stitches and reading the code for the first time

Maybe we could add a method or something similar that more explicitly tells the user what's going on behind the scenes.
We will think this through and get back and discuss things here with you

LucasUnplugged · 2022-02-14T19:44:44Z

That makes sense, @hadihallak. Something like this would indeed be clearer:

const Button = styled('button', {...}).named('Button').

I'm not even sure if that's (realistically) possible, so didn't even attempt it.

guipolitano · 2022-02-14T20:58:24Z

Hope this PR gets merged soon

@guipolitano Sadly, i don't think it's possible to get this merged as is as the API conflicts with another stitches API which is composing n number of styles into one component
const Button = styled('button', {...styles}, {...morestyles}, anotherStyledComponent)
To be clear, We love this feature and we know how important it is for the reasons outlined in the PR but we just need to think more about the API in order to maintain the DX that everyone loves about Stitches

We will think more about this and hopefully come back with a plan/decision for the API sometime next week and discuss it with @LucasUnplugged and everyone in here.

Sadly =/.
Maybe it wouldn't be better to use babel-plugin, just as Styled-Components do? I had try to create one, but i know nothing about it so I couldn't achieve it.

hadihallak · 2022-02-14T21:09:30Z

@guipolitano
yeah that's actually one of the main reason we need this feature. to enable us to automate the process of creating stable hashes and friendly classNames which is currently not possible as we have no API underneath to support it yet

guipolitano · 2022-02-14T21:19:39Z

@guipolitano yeah that's actually one of the main reason we need this feature. to enable us to automate the process of creating stable hashes and friendly classNames which is currently not possible as we have no API underneath to support it yet

Understood, hope you guys find a way. Thx for the great job, im loving to use Stitches with Radix. :)

hadihallak · 2022-06-06T16:53:50Z

Closing in favor of #1005

bartcheers · 2023-03-19T05:15:25Z

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.

LucasUnplugged added 4 commits December 12, 2021 10:46

fix(878): Fixed issue with ThemeTokens typing.

e83f8c4

fix(878): Applied PR review comments.

ac35f5f

fix(749): Resolved type issue when using as & variant props together.

73646c6

fix(650): Added ability to define friendly class names.

fba8bb5

Squashed commits: [a3e0552] [0a1cdd9]

LucasUnplugged requested a review from peduarte as a code owner January 3, 2022 12:44

LucasUnplugged mentioned this pull request Jan 3, 2022

Generate friendly class names #650

Closed

luixo mentioned this pull request Jan 19, 2022

Stitches: proof of concept near/near-explorer#862

Merged

luixo reviewed Jan 25, 2022

View reviewed changes

feat(650): Added tests for friendly class names.

72d7c8c

hadihallak self-assigned this Feb 9, 2022

LucasUnplugged mentioned this pull request Apr 16, 2022

Add support for custom (human-friendly) class names #1005

Closed

hadihallak closed this Jun 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for #650: Generate friendly class names #916

Fix for #650: Generate friendly class names #916

LucasUnplugged commented Jan 3, 2022

codesandbox-ci bot commented Jan 3, 2022 •

edited

Loading

LucasUnplugged commented Jan 24, 2022

luixo left a comment

LucasUnplugged commented Jan 25, 2022

luixo Jan 25, 2022

LucasUnplugged Jan 29, 2022

LucasUnplugged Jan 29, 2022

luixo Jan 31, 2022

LucasUnplugged commented Jan 29, 2022

guipolitano commented Feb 14, 2022

hadihallak commented Feb 14, 2022

LucasUnplugged commented Feb 14, 2022

hadihallak commented Feb 14, 2022

LucasUnplugged commented Feb 14, 2022

guipolitano commented Feb 14, 2022

hadihallak commented Feb 14, 2022

guipolitano commented Feb 14, 2022

hadihallak commented Jun 6, 2022

bartcheers commented Mar 19, 2023

Fix for #650: Generate friendly class names #916

Fix for #650: Generate friendly class names #916

Conversation

LucasUnplugged commented Jan 3, 2022

Proposed API

Related Discussion

codesandbox-ci bot commented Jan 3, 2022 • edited Loading

LucasUnplugged commented Jan 24, 2022

luixo left a comment

Choose a reason for hiding this comment

LucasUnplugged commented Jan 25, 2022

luixo Jan 25, 2022

Choose a reason for hiding this comment

LucasUnplugged Jan 29, 2022

Choose a reason for hiding this comment

LucasUnplugged Jan 29, 2022

Choose a reason for hiding this comment

luixo Jan 31, 2022

Choose a reason for hiding this comment

LucasUnplugged commented Jan 29, 2022

guipolitano commented Feb 14, 2022

hadihallak commented Feb 14, 2022

LucasUnplugged commented Feb 14, 2022

hadihallak commented Feb 14, 2022

LucasUnplugged commented Feb 14, 2022

guipolitano commented Feb 14, 2022

hadihallak commented Feb 14, 2022

guipolitano commented Feb 14, 2022

hadihallak commented Jun 6, 2022

bartcheers commented Mar 19, 2023

codesandbox-ci bot commented Jan 3, 2022 •

edited

Loading