Dynamic + attr + custom element behavior is very erratic

Blankeos commented 2 weeks ago

Describe the bug

2 SolidJS concepts colliding with eachother:

Dynamic
Attr

For instance when using this code:

      <Dynamic component={"my-custom-element"} data-awesome="true">
        1. I'm a dynamic component without `attr:*`. Inspect element me. Expect:
        No attributes.
      </Dynamic>

      <br />
      <br />

      <Dynamic component={"my-custom-element"} attr:data-awesome="true">
        3. I'm a dynamic component with `attr:*`. Inspect element me. Expect:
        Has attribute.
      </Dynamic>

There's currently 3 unexpected behaviors that are happening when using the code above.

Your Example Website or App

https://github.com/Blankeos/mre-solid-attr-bug

Steps to Reproduce the Bug or Issue

Clone the repo https://github.com/Blankeos/mre-solid-attr-bug.git
npm run dev
Check the elements in inspect element.
When done, go to app.config.ts, and ssr: true
Check the elements in inspect element.
When done, try checking when instead of "my-custom-element", this time just "div".
Check the elements in inspect element.

Expected behavior

attr:data-something and the regular data-something to have the same results.
Using attr:* on the server should not put attr:something on the rendered html as well.

Screenshots or Videos

No response

Platform

OS: macOS
Browser: Arc
Version: 1.63.1 (54714)

Additional context

No response

Blankeos commented 2 weeks ago

Also just sharing, my best workaround for this right now is to manually tweak it into the desired result at onMount:

  onMount(() => {    
    const el = props.ref as unknown as HTMLElement;
    const _my_attribute = el.getAttribute('attr:my-attribute');
    if (_my_attribute) {
      el.removeAttribute('attr:my-attribute');
      el.setAttribute('my-attribute', _my_attribute);
    }
  });

Basically, always use attr: especially when passing signals to them.

titoBouzout commented 1 week ago

@Blankeos Thanks for your report! A PR for fixing this is in https://github.com/ryansolid/dom-expressions/pull/371

Just so you know, custom-elements in solid use properties instead of attributes. So the case of the first screenshot is working as expected, and the other cases are included as a fix in the PR.

This behaviour is currently being re-evaluated, the consensus reached by the people using custom-elements was to use properties instead of attributes. As you can imagine changing this will be a breaking change, and requires some considerable effort.

Blankeos commented 1 week ago

Thank you for the quick response @titoBouzout. Thanks for the info. Yeah I hope it gets fixed, but definitely no rush since I have a workaround for my use-case right now. You guys are doing amazing work.

Btw for context: I'm currently porting a library called https://solid-number-flow.pages.dev. I think the base library number-flow uses the parts="" attribute in the custom element <number-flow> to work.

ryansolid commented 1 week ago

I believe this is fixed in the latest (1.9.3).. minus the first panel which is expected behavior.

solidjs / solid