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

Add ability to automatically forward all attributes to props #34

Closed
patricknelson opened this issue Dec 4, 2023 · 1 comment · Fixed by #35
Closed

Add ability to automatically forward all attributes to props #34

patricknelson opened this issue Dec 4, 2023 · 1 comment · Fixed by #35
Assignees
Labels
enhancement New feature or request

Comments

@patricknelson
Copy link
Owner

patricknelson commented Dec 4, 2023

Describe the problem

For you to ensure that any particular custom element attribute update is forwarded to the associated Svelte component prop, with svelte-retag one must explicitly define each prop in the main attributes array. However, with Svelte 4 this is not necessary (thanks to the benefits of the compiler already numerating props ahead of time).

This is now possible ever since context support was added in issue #10 (by the magic of Proxy), we now already fully aware of all necessary props ahead of time without having to require the developer to explicitly configure each one.

Main benefits

  • Maintain parity with how Svelte 4+ implements custom elements, therefore reducing confusion for new adopters.
  • Not doing this results in increased required work for developers who need to automatically push changes to element attributes to their Svelte components
  • For those needing forwarded attributes, not having this will get in the way of using automatic definition of custom elements via tools like Vite's import.meta.glob feature. Adding automatic forwarding of changes (like Svelte 4 already does) means developers can have both (without having to decide between manually defining vs. automatic defining)

Potential caveats

  • This could potentially be less efficient than attributeChangedCallback since we'd be notified of every attribute change and would need to manually filter out in the MutationObserver listener.
  • This should theoretically not be a breaking change. However, there may exist some possible issues, depending on developer code outside of svelte-retag. For example, there could be edge cases in developer code where attributes on custom elements update unexpectedly or frequently after initialization (unrelated to svelte-retag), but because svelte-retag doesn't currently forward all prop-attributes to props, it doesn't cause any problems.

Conclusion

IMHO, the known benefits of reduced overhead, consistency with Svelte and potential for automated definition outweigh the potential caveats listed above.

Describe the proposed solution

Move to using MutationObserver instead of observedAttributes(). This is required since observedAttributes() is static, but we simply cannot automatically know at this state what the attributes will be ahead of time. We can only dynamically infer props at runtime (after the component has been instantiated).

Backward compatibility

v1:

For v1, to prevent any API level regressions, attributes will function as before but instead will be extended to also allow boolean. So, the existing default of [] will remain, but the new true value can be passed to enable automatic forwarding of all attributes to component props (and then will be encouraged).

v2:

In Svelte 4, there is no option to disable automatic forwarding of all attributes to component (which would normally be exposed via the newer <svelte:options customElement={…} /> API). Also, since we're aligning with that API and the option isn't present, it makes sense to just make this the default and to do this in v2 (as this is backward incompatible).

A new issue will be created (TBD) to address this (TBD), since this one is for the work to add it in v1.

Alternatives considered

None.

Importance

would make my life easier

@patricknelson patricknelson added the enhancement New feature or request label Dec 4, 2023
@patricknelson patricknelson self-assigned this Dec 4, 2023
@patricknelson patricknelson added this to the v2 milestone Dec 4, 2023
@patricknelson
Copy link
Owner Author

Even though I want to release earlier than later, slated for now as a v2 change since it would technically be a backward incompatible API change (attributes defaults to [] and not null and defaults to not forwarding anything at all).

patricknelson added a commit that referenced this issue Dec 4, 2023
…ributes" option, allowing ability to forward all attributes in a v1 compatible fashion.
@patricknelson patricknelson changed the title Automatically forward attributes to props (mitigate need for legacy attributes option) Add ability to automatically forward all attributes to props Dec 5, 2023
@patricknelson patricknelson removed this from the v2 milestone Dec 5, 2023
patricknelson added a commit that referenced this issue Dec 5, 2023
…ributes" option, allowing ability to forward all attributes in a v1 compatible fashion.
patricknelson added a commit that referenced this issue Dec 5, 2023
patricknelson added a commit that referenced this issue Dec 5, 2023
…and experimental "autoDefine()" function (due to large number of components). Undocumented for a reason... please do not use in production! 😅
patricknelson added a commit that referenced this issue Dec 5, 2023
patricknelson added a commit that referenced this issue Dec 6, 2023
…ributes" option, allowing ability to forward all attributes in a v1 compatible fashion.
patricknelson added a commit that referenced this issue Dec 6, 2023
patricknelson added a commit that referenced this issue Dec 6, 2023
…and experimental "autoDefine()" function (due to large number of components). Undocumented for a reason... please do not use in production! 😅
patricknelson added a commit that referenced this issue Dec 6, 2023
patricknelson added a commit that referenced this issue Dec 6, 2023
…ributes" option, allowing ability to forward all attributes in a v1 compatible fashion.
patricknelson added a commit that referenced this issue Dec 6, 2023
patricknelson added a commit that referenced this issue Dec 6, 2023
…and experimental "autoDefine()" function (due to large number of components). Undocumented for a reason... please do not use in production! 😅
patricknelson added a commit that referenced this issue Dec 6, 2023
patricknelson added a commit that referenced this issue Dec 6, 2023
…onverting to using querySelector instead of the old original naive HTML comparison tests.
patricknelson added a commit that referenced this issue Dec 6, 2023
patricknelson added a commit that referenced this issue Dec 6, 2023
…as expected even if element is disconnected from DOM and reconnected again later.
patricknelson added a commit that referenced this issue Dec 7, 2023
…s to Svelte component using "attributes: true" (via #34)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant