Skip to content

Fix boolean attributes in presence of spread attributes #3775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 23, 2019

Conversation

Conduitry
Copy link
Member

Fixes #3764, with the assumption that #3750 may mean that it would become even more impractical to ship attribute-to-property tables along with the runtime.

In short:

  • For each boolean attribute (more precisely, for each attribute with attribute_lookup metadata) given for a node that has at least one spread, the compiler will emit the appropriate property key in the object in the spread arrays. For example, { readOnly: whatever } instead of { readonly: whatever }.
  • In SSR mode, the corresponding object in the spread array now contains { readonly: whatever || null } instead of {readonly: whatever } for boolean attributes, to ensure that the spread() function does not serialize falsy attributes.

Bonus SSR bugs I bumped into and fixed while addressing this:

  • {...{ foo: null }} was getting serialized as foo="null" because spread() was checking for strict equality with undefined. It's now checking for loose equality with null.
  • Attributes had to be lowercase for the special logic for value, class, or any boolean attribute to take effect. These checks are now done case-insensitively.

@Conduitry
Copy link
Member Author

Bonus bonus fix (thanks @mrkishi):

<input placeholder='foo' {...{ placeholder: null }}>

now produces <input> rather than <input placeholder='null'>.

@Conduitry
Copy link
Member Author

gh actions seem to be having a bad time. I'll re-run later today and see if I can get that ol green checkmark before merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spread properties on inputs have unexpected readonly attributes
1 participant