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

classname utility proposal #78

Open
geoctrl opened this issue Mar 3, 2024 · 1 comment
Open

classname utility proposal #78

geoctrl opened this issue Mar 3, 2024 · 1 comment
Assignees
Labels

Comments

@geoctrl
Copy link
Contributor

geoctrl commented Mar 3, 2024

I've been hard at work moving kremling over to a monorepo (i'll write another issue here about that soon), and it's really close! the last thing left is figuring out what to do with the classname utilities: always() maybe() toggle()

Background

This utility uses a string object instead of a string primitive so we can pass values around and make it chainable. It works, albeit there’s some weird caveats like sometimes requiring toString() if you’re using prop types, or if you’re working in other libraries like solidjs, this solution just flat out fails.
Also, typing it with typescript is a major headache - i’ve spent too much time arguing with chatgpt about the best approach to solving this... tldr: we’d have to assert the type every time we use it, or we use toString() at the end of every implementation 🤮

option 1 - leave it be

we keep it as-is and don’t convert it to typescript - also keep the caveats I mentioned above ^ - this works because the type definitions that were manually written for it extend the output of each method with & String which satisfies react’s className type

option 2 - proposal:

we rewrite it:

className={a('one')}
className={m(condition, 'two')}
className={t(condition, 'three', 'four')}

this looks very similar to the old way… except when we compose them together - instead of chaining, we pipe them in with always:

className={a('one', m(condition, 'two'), t(condition, 'three', 'four'))}

the always method is crucial because it allows infinite string arguments: a(...strArgs: string[]): string joining all the strings together leaving a simple string primitive!

benefits:

  • output for all methods is always a string primitive, you’ll never need to toString()
  • no chaining
  • can trivially write and test in typescript
  • makes splitting out lots of utility classes readable
  • only import the methods you need - if you never use t(), it’ll be tree-shaken out

downsides:

  • you don’t automatically get all methods by importing one of them… each method will need to be imported like rxjs’s pipe methods…
  • using 2 or more methods without the need of always still requires always to combine them: a(m(), m()) (although i would argue that not using a() is an edge case in this scenario)

BONUS QUESTION:

do we keep classname helpers in kremling? is this useful enough to pull out and make it's own library?

@geoctrl geoctrl self-assigned this Mar 3, 2024
@geoctrl
Copy link
Contributor Author

geoctrl commented Mar 3, 2024

here's a first-pass at the new utilities:

export const css = (...args: (string | undefined)[]): string => {
  return args.filter(Boolean).join(" ");
};

export const maybe = (
  enabled: boolean | undefined | null,
  ...className: string[]
): string => {
  return enabled ? className.join(" ") : "";
};

export const toggle = (
  enabled: boolean | undefined | null,
  className1: string | string[],
  className2: string | string[],
): string => {
  const one = Array.isArray(className1) ? className1.join(" ") : className1;
  const two = Array.isArray(className2) ? className2.join(" ") : className2;
  return enabled ? one : two;
};

export { css as c, maybe as m, toggle as t };

and usage examples:

import { a, m, t } from "@kremlingjs/react";

<div className={a(
  "one",
  m(condition, "two"),
  t(condition, "three", "four"),
)} />

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

No branches or pull requests

1 participant