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

fix: Allow to use bind:property on custom elements #11427

Open
wants to merge 1 commit into
base: svelte-4
Choose a base branch
from

Conversation

ouvreboite
Copy link

πŸ› Related issue #4838

Hello Svelte team,

Currently a compilation error is thrown when trying to bind on a (valid or invalid) property from a custom element (aka web component).
For example:

<my-custom-element bind:value={myValue}></my-custom-element>
<!-- The compiler returns a validation error: 'value' is not a valid binding on <my-custom-element> -->

The goal of this PR is to propose a naive (and simple) fix for Svelte 4. A more robust solution can be implemented in Svelte 5, but some design discussion may be necessary.

The fix returns a warning (instead of an error) when using bind:property on custom elements. bind:this is still supported (no error or warning).

❓ πŸ†˜ How to test properly?
I was able to adapt the compilation check easily and create a test case for the new warning being returned. But I don't know how to implement a proper test (using an actual web component, binding to one of its property and checking that the binding works). I welcome some feedback on that.

πŸ§ͺ New test case for compilation validator

<script>
	let myValue;
	let myElement;
</script>

<my-element bind:value={myValue} bind:this={myElement}></my-element>

<!-- will return no error -->
<!-- will return a single warning: Binding on custom element <my-element> is not checked. You need to ensure 'value' is a valid property. -->

Copy link

changeset-bot bot commented May 2, 2024

⚠️ No Changeset found

Latest commit: f144e2f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dummdidumm
Copy link
Member

Thanks, but I'm not sure this is a good solution. It assumes that the bindings on custom elements behave exactly like those on the native dom elements they were originally designed for, which may not be the case. People will likely get confused by it not working, and custom element authors would now be on the hook for somehow adjusting their custom elements to account for someone using a binding. We framework authors would now also have to be extra careful about not assuming a native dom element or not using private properties as much.

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.

None yet

2 participants