Skip to content
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

Cannot use well-known attribute names with custom elements #45

Closed
4 tasks done
hesxenon opened this issue Dec 3, 2024 · 5 comments · Fixed by #46
Closed
4 tasks done

Cannot use well-known attribute names with custom elements #45

hesxenon opened this issue Dec 3, 2024 · 5 comments · Fixed by #46
Labels
💪 phase/solved Post is done

Comments

@hesxenon
Copy link
Contributor

hesxenon commented Dec 3, 2024

Initial checklist

Affected package

https://esm.sh/hast-util-to-html@9?bundle

Steps to reproduce

See here: https://jsbin.com/jelewudote/3/edit?html,console,output

Copy Pasta'd repro for later reference
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>JS Bin</title>
</head>
<body>
  <script>
    console.clear();
    class Foo extends HTMLElement {
      connectedCallback() {
        console.log(`rendered: ${this.getAttribute("selected")}`);
      }
    }
    customElements.define("my-foo", Foo)
  </script>

  <script type="module">
    import {toHtml} from "https://esm.sh/hast-util-to-html@9?bundle"

    console.log(`serialized: ${toHtml({
      type: "element",
      children: [],
      properties: {
        selected: "some value",
      },
      tagName: "my-foo",
    })}`)
  </script>
  
  <my-foo selected="some value"></my-foo>
</body>
</html>

Basically:

  1. define a custom element which relies on the value of an attribute that has a well-known name
  2. serialize such a custom element with an "unexpected" value for the well-known name

Actual behavior

Serialization enforces the well-known definition over its intended usage.

"serialized: <my-foo selected></my-foo>"

Expected behavior

Serialization should not enforce the well-known definition in the case of custom elements. It should align with what is rendered by the browser.

"serialized: <my-foo selected="some value"></my-foo>"

Runtime

Firefox/Chrome

Package manager

No response

Operating system

No response

Build and bundle tools

No response

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Dec 3, 2024
@wooorm
Copy link
Member

wooorm commented Dec 4, 2024

Could you take a second to look at which exact package the problem is? Might not be this exact package. If it is this package, a broken test is nice.

Though, I'd think:

  • Weird values for a Boolean should indeed be passed through as is
  • Actually select=select, I can see that being turned into select, which I think custom elements should support?
  • q: what do you think about such attributes on known elements? Like div or button?

@hesxenon
Copy link
Contributor Author

hesxenon commented Dec 4, 2024 via email

@wooorm
Copy link
Member

wooorm commented Dec 5, 2024

Right so what I am thinking is: if some value is a string (asd) where a boolean is normally expected (say for hidden), we can still serialize that string here (hidden=asd)

Whether specific custom element handling is needed in this (or other) projects can then be ignored/delayed. This might be a can of worms, because it is vague whether class for example is a space separated list. It will be for some things (CSS). It might not be for the code handling the custom element. That’s what I worry about.

If serializeAttribute is the only thing causing your problem, then it’s not too hard to solve. Are you interesting in working on a PR?

@hesxenon
Copy link
Contributor Author

hesxenon commented Dec 5, 2024 via email

@wooorm
Copy link
Member

wooorm commented Dec 13, 2024

Released in 9.0.4!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done
2 participants