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

[slottable-request] Given that remove is a short, common identifier, we may want to consider exposing this as a static symbol like SlottableRequestEvent.remove. #56

Open
Westbrook opened this issue Apr 4, 2024 · 3 comments

Comments

@Westbrook
Copy link
Collaborator

          Given that `remove` is a short, common identifier, we may want to consider exposing this as a static symbol like `SlottableRequestEvent.remove`.

Originally posted by @hunterloftis in #45 (comment)

@justinfagnani justinfagnani changed the title Given that remove is a short, common identifier, we may want to consider exposing this as a static symbol like SlottableRequestEvent.remove. [slottable-request] Given that remove is a short, common identifier, we may want to consider exposing this as a static symbol like SlottableRequestEvent.remove. Apr 10, 2024
@tbroyer
Copy link
Contributor

tbroyer commented Jan 13, 2025

Fwiw, currently it's defined as remove = Symbol('slottable-request-remove'), but that makes it non-interoperable between distinct implementations of the slottable-request protocol.

I think you meant to use remove = Symbol.for('slottable-request-remove') to share the same symbol over all implementations.

> Symbol("slottable-request-remove") === Symbol("slottable-request-remove")
// → false
> Symbol.for("slottable-request-remove") === Symbol.for("slottable-request-remove")
// → true

Similarly, using a static field meant to be used as SlottableRequestEvent.remove moves the concern to the implementation of the SlottableRequestEvent implementation being used by the consumer (which might not be the one of the event you're actually handling, used by the custom element).

This could be solved with either an instance field (if (event.data === event.remove)), a specific instance readonly property (if (event.remove)), or possibly a static field but documented as event.constructor.remove to be implementation-agnostic; or as said above using Symbol.for("slottable-request-remove")

@Westbrook
Copy link
Collaborator Author

Great catch. Happy to accept a PR on either the general naming or the symbol structure! 🙇🏼

@justinfagnani
Copy link
Member

Yeah, the proposal currently defines this like there would be a common module that implementations could share. I think we'll want to do a pass to re-define things in terms of interfaces only. Symbol.for() is an improvement, but like a lot of DOM APIs, I think I'd probably prefer strings if they work.

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

No branches or pull requests

3 participants