Open samuelgozi opened 6 months ago
If this PR is approved I have no problem documenting it in the official Solid docs.
I thought about it and maybe in order to support objects in the array we can flatten nested object into the returned object from classListToObject
.
It will essentially work like Object.assign
, so it will be easy to reason about if multiple objects contain the same keys, and otherwise functionality still remains the same.
Then the type of ClassList changes to:
type ClassList =
| Record<string, boolean>
| Array<
string | number | boolean | null | undefined | Record<string, boolean>
>;
So nested objects inside objects are not allowed.
Edit: Added this capability and edited the PR description.
@ryansolid What do you think?
This lacks the compiler optimization step for classList, might be needing that if we were to approve this
Can you give me some directions on how to do that, or where it is?
Yeah this doesn't look at the compiler side of things. It's possible we just need a heuristic to bail out of the optimization I suppose. Like if we see an array, or some of the other values.
I'm not going to lie classLIst's future is still up in the air. It is one of the things we know for sure that will be reviewed in 2.0. So this might drag a bit until we can figure out how to proceed.
I saw that there was a somewhat similar PR https://github.com/ryansolid/dom-expressions/pull/88, however this PR differs from it by not trying to support nested objects inside an array.
Edit: I added support for one level of nested objects into the array. I updated the description to reflect that.
I purposely didn't want to change too much since I wanted this change to be small and incremental. Perhaps in the future, if I see that nested objects inside an array is indeed a nice API I might volunteer to add it.
This PR is only additive, and doesn't change any existing behavior.
The specific implementation in this PR is very short and doesn't change the existing logic at all for the existing syntax for objects. However it adds one more loop for
classList
Arrays in order to convert them into an object, which is then treated the same as it does before this PR. If you don't like this approach I can offer an alternative.I opened a discussion about this here with some examples: https://github.com/solidjs/solid/discussions/2036
I also added tests with all relevant examples.
You can play with and test the implementation here (open the console): https://stackblitz.com/edit/typescript-jbx9jn
General logic overview
classList
now supports two types of arguments:Record<string, boolean>
(Same as before)Array<string | number | boolean | null | undefined | Record<string, boolean>>
Objects are not allowed to have Arrays or nested objects, only booleans. Arrays can have objects with booleans, but not arrays, nor objects with objects. This keeps implementation and logic easy to reason about.
When
classList
is called with an array, it will convert the array into an object while skipping all false values except0
. This has the side effect of leavingtrue
as an object key, but that is also what happens in VueJS, so I consider that OK.Edit: Now if the array contains objects, it will "flatten" them in the order they were passed. The behavior is like the spread operator.
Changes are then handled as if it was a regular object, so it essentially works exactly like passing an object with all values set to
true
.Examples
Strings
Booleans
Undefined
Null
Numbers
Objects in the array