2.34.x is a breaking change, should be 3.0.x

[vimtaai]

Hi,

We faced an issue when upgrading to 2.34 version of hyperhtml. The new behaviour with the ?disabled=${value} option causes breaking changes when using the null value.

Previous behaviour: disabled=${null} sets the disabled property on the DOM element to false
Current behaviour: disabled=${null} sets the disabled property on the DOM element to true

Because of this breaking change in behaviour the major version should be bumped to 3.x.x to indicate the breaking change.

[WebReflection]

wait ... the breaking change wasn't meant, and ?disabled=${value} is opt-in, not mandatory ... are you using ?disabled or just disabled?

[WebReflection]

to clarify, nothing touches the previous behavior here and that's what went out

[vimtaai]

We used it without the ?. Several of our integration tests broke because of the upgrade.

[WebReflection]

From which version?

[vimtaai]

Just a quick check on the code:
Line 1501 in index.js has a single = in the if condition. Was this intentional?

[WebReflection]

You don’t reach that code and yes, double (( )) means intentional

[WebReflection]

ac14025#diff-a1e35bf752b708a7f4ff7570d938ff532b7c714aa45ca92dd6f9b5dae70a677aR56

[WebReflection]

Don’t look at anything that is not under esm folder, just to clarify

[vimtaai]

Just trying to make sense of the code, maybe I'm completely off track:

hyperHTML/index.js

Lines 1622 to 1628 in 7a89190

	if (newValue == null) {
	if (owner) {
	owner = false;
	node.removeAttributeNode(attribute);
	}
	attribute.value = newValue;

If I'm correct this is the snippet that runs for using disabled=${null}. In this case shouldn't the function return after removing the attribute on line 1625? It seems to be contradictory to remove an attribute then assigning a new value to it on line 1628.

[WebReflection]

I've tested the code (as there are a lot of tests, we use this in production) and I cannot reproduce the issue ... can you reproduce the issue? Again, nothing changed in here in 2.34, you never reach changed code if there's no ?disabled=${...}

[WebReflection]

also, like I've said, I won't comment on anything that is not under the esm/ folder, as nothing else matters.

[WebReflection]

last, but not least, the attribute value is syncd'

if (attribute.value !== newValue) {

can you help me reproducing this issue? I think you have something else going on there ... I can't reproduce this issue.

[vimtaai]

I'll try to reproduce it.

[WebReflection]

anyway, this is the only change and nothing else changed ... so I am pretty confident you have some other issue, or somebody started passing along different values (not null) somewhere else.

[vimtaai]

We were upgrading from version "2.18.0". Could the issue lie somewhere else? 🤔