Spreads: Property new/old value equality check isn't done against the DOM value

[AFatNiBBa]

Describe the bug

If, on the playgrround example, the line containing the spread gets removed, the value property of the input field will be assigned as follows:

_$effect(() => _el$.value = get());

This will have the desired effect. If the spread line doesn't get removed the value property will be setted like this:

_$spread(_el$, _$mergeProps({
  get value() {
    return get();
  }
}, {}), false, false);

The spread() function sets properties of elements using assignProp(), which won't change the value of the property on the actual element if its old value is the same as the new one. The problem is that if we type "unallowe" and then add a "d", the signal will remain with "unallowe" while the input field will contain the "d" too. This happens because the value property of the actual element contained the "d" because of the user interaction, but the solid props used to create that input field still only contained "unallowe"; so when the signal was updated with the same value ("unallowe"), assignProp() believed that there were no changes. I think this line should not return if value is different from the one contained in node

Notice that there's a workaround for this problem, you just need to set the prop:value attribute instead of the value one

Your Example Website or App

https://playground.solidjs.com/anonymous/6e6f686b-e624-49c5-b5cb-f445207883f4

Steps to Reproduce the Bug or Issue

1. Type "unallowe" on the input field
2. Type "d", the new letter will be shown but the actual value will remain the same
3. Type another character, the actual value will start being updated again

Expected behavior

When typing "d" (2nd step), nothing should've turned up in the first place

Screenshots or Videos

No response

Platform

(Not needed)

• OS: Windows
• Browser: Edge
• Version: 120.0.2210.77

Additional context

No response

[ryansolid]

That's right at some point we special cased value in compilation but not in spread it appears. Interesting though that prop:value works. Worth looking at. There are definitely some attribute values you want guarded so safer general approach is to do the equality check like we do. However reading from the DOM comes with other consequence so I'd prefer to avoid that at all costs.

[ryansolid]

Ungrouping could cause otherside effects. I could remove the check for value and checked but it would always reflect. Like any change to a prop on the element would overwrite it potentially. It wouldn't be hard to implement but I'm concerned about edge cases. That being said it is a bit inconsistent given we've special cased other scenarios (which is always the problem when you special case something).

As for prop:value it seems this worked because we weren't combining it into the spread which is an interesting choice I admit. On one hand they should be because spread can handle it in the sense it can apply it. But on the other hand merging with non prefixed versions wouldn't really work predictably anyway (last run wins). Which really is the issue with having attributes/properties being able to be set multiple ways.

[AFatNiBBa]

Sorry to bother again, but there is still a problem, this line

if (value === prev && (prop !== "value" || prop !== "checked")) return prev;

Should be

if (value === prev && prop !== "value" && prop !== "checked") return prev;

Since the (prop !== "value" || prop !== "checked") part can only return true

[ryansolid]

Yep that was sloppy. I should have written a test.

It will make its way into the next Solid release: https://github.com/ryansolid/dom-expressions/commit/e35e15db731acff289bdc96528d3d48048635511

[ryansolid]

One of those other side effects showed up and it is equally inconsistent so I'm going to err on the side of not breaking existing behavior and we will have to find a different better fix for this or come up with a different expectation as neither the fix nor checking the DOM node will be sufficient here. So I'm reverting the fix and labeling this for a future release.