solidjs / signals

MIT License
144 stars 4 forks source link

`createSelector` when used with multiple values #11

Closed titoBouzout closed 3 weeks ago

titoBouzout commented 5 months ago

There seems to be some sort of bug with createSelector when It's used with multiple values, there are many reports of not being O(1)

I understand the purpose of this primitive, is to run effects for the values that change instead of running effects for all the values to see what have changed.

For example on this case, when you add to items, it seems like iterates all the items? https://playground.solidjs.com/anonymous/f4e9e5a4-e141-477c-9d0c-3e7bc3392a79

This doesn't seem to be the case when the value is a simple value/not an iterable/not an array.

titoBouzout commented 5 months ago

Do we really need a comparator function?

I am not sure that we require that, and in any case users can write their own stuff.

The comparator function seems to make the default/most used result worse, as it has to run a user supplied function for all values.

I would also say to write it with regular apis, as follows: https://github.com/potahtml/pota/blob/master/src/hooks/useSelector.js

Some fabio insights on https://discord.com/channels/722131463138705510/751355413701591120/1249864811756654747

ryansolid commented 3 weeks ago

Since we are getting rid of createSelector I believe this will no longer be a concern.