jorgebucaran / classcat

Build a class attribute string quickly
MIT License
905 stars 22 forks source link

Remove support for nested objects. #15

Closed jorgebucaran closed 6 years ago

jorgebucaran commented 6 years ago

In retrospect, I shouldn't have added a feature I didn't need at the time (YAGNI), but I wanted one feature more than classNames. I still don't need this feature and think it's unnecessary bloat, so I'd like to remove it and release a 3.0 (even though I just published 2.0 with a breaking bug fix from @developerdizzle).

EDIT: If people using this project are opposed to this change, I am okay with not removing it, otherwise let's do it! 🔥

developerdizzle commented 6 years ago

Just to confirm, you would support this:

cc([
    "foo",
    {
        "--bar": true
    }
])

but not this:

cc([
    "foo",
    {
        "-bar": {
            "--baz": true
        }
    }
  }
])
jorgebucaran commented 6 years ago

@developerdizzle The first one's output would be foo --bar. Note the space.

developerdizzle commented 6 years ago

So in order to support what you can currently do with this:

cc([
    "foo",
    {
        "foo": {
            "--bar": true,
            "--baz": false,
            "--quz": true
        }
    }
  }
])

you would instead do this:

cc([
    "foo",
    {
        "foo--bar": true,
        "foo--baz": false,
        "foo--qux": true
    }
  }
])
developerdizzle commented 6 years ago

How much of a size/performance improvement do you think that would gain?

jorgebucaran commented 6 years ago

@developerdizzle Some, but another reason is that I just don't need it and feel it complicates, not the code, but how you can use the library a lot (e.g., it's difficult to document properly).

What do you think?

developerdizzle commented 6 years ago

Personally, I do like the

        "foo": {
            "--bar": true,
            "--baz": false,
            "--quz": true
        }

syntax. Realistically, if you wanted to, you could also remove arrays by forcing the user to use a single object, like this:

cc({
    "foo": true,
    "foo--bar": true,
    "foo--baz": false,
    "foo--qux": true
})

and make things even less complicated, but I'm not sure where exactly you draw the line.

vdsabev commented 6 years ago

I voted for removing, it's a bit of black magic that you don't really need anyway. Here's my suggestion:

classcat([
  "foo",
  {
    "--bar": true,
    "--baz": false,
    "--qux": true
  }
])

Then define --bar, --baz, and --qux unprefixed in your CSS. What, do you have global classes with -- or something? 😅

This is also described in https://css-tricks.com/abem-useful-adaptation-bem - I don't necessarily agree with all of the arguments in the article, but using --modified-name without the prefix has made my life much easier.

Example with Angular:

<div
  class="builder__component"
  [class.--hovered]="hovered"
  [class.--selected]="selected"
  [ngClass]="[
    '--level-' + level,
    '--is-' + component.name
  ]"
></div>

Imagine if I had to prefix each one of those modifiers with builder__component...tedious 😫

jorgebucaran commented 6 years ago

@MatejMazur A quick search revealed you were using the feature in your project here.

<Cell
  key={property.id}
  className={cc([
    {
      TableFormCell: {
        [`--${property.id}`]: true,
        '--readOnly': readOnly,
        '--input': !!property.type
      }
    },
    ...classNames
  ])}
>

...and without support for nested objects:

<Cell
  key={property.id}
  className={cc([
    {
      [`TableFormCell--${property.id}`]: true,
      "TableFormCell--readOnly": readOnly,
      "TableFormCell--input": !!property.type
    },
    ...classNames
  ])}
>
jorgebucaran commented 6 years ago

💁‍♂️ https://github.com/JorgeBucaran/classcat/releases/tag/3.0.0