-
Notifications
You must be signed in to change notification settings - Fork 158
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
Symbols or strings for Temporal.Calendar and Temporal.TimeZone methods? #310
Comments
This comment has been minimized.
This comment has been minimized.
I don't have a strong opinion on string vs. symbol. If TC39 wants to eventually move in the direction of symbol-based protocols, then we should use symbols. But if TC39 doesn't intend to move forward with that proposal, then we should use strings. @michaelficarra? Comments? |
Currently, I don't think TC39 has decided on whether to move forward with the protocol proposal. I was hoping we could decouple these, and make Temporal work whether or not protocols eventually happen in the proposed form. I'm looking forward to @michaelficarra's comments. |
The language has both String and Symbol based protocols, I don't think the first-class protovol proposal would force one or the other (since imo it needs to work with strings, too, to represent thenables/tostringables/valueofables/tojsonables) |
@ljharb I'm not sure if representing those things is a goal of the protocol proposal. |
No, but it’s a requirement for advancement i have that ive made clear on that proposal repo. |
I'm in favour of keeping methods as strings. While these are protocols as well, they have direct value being called directly by programmers as well. Symbol methods seem strange in that context. To my mind (and I might well be wrong) symbol-protocols are useful to allow implementation by random objects. I'm thinking of examples like iteration; random objects can become iterable by implementing a Symbol based protocol. TimeZone & Calendar seem much more direct. Some random object couldn't just turn into a timezone. That just seems a bit unintuitive. |
The first-class protocols proposal includes requiring but not providing string-named properties as part of a protocol. See this slide from the most recent presentation to the committee.
TC39 has already been using symbol-based protocols. This is in no way dependent on my proposal, the purpose of which is to represent these protocols as a value that you can refer to and pass around. If you've identified a protocol that developers will be using to intentionally coordinate with built-ins or with other developers, it should be symbol-based. This is especially the case when you're considering protocols with a method named something very generic like "plus" and objects that implement that protocol may need to implement other protocols. |
We've decided not to do this with Promises and RegExp subclassing. Do you see these as mistakes not to be repeated?
It could be useful to think through cases where this occurs, to understand how serious the issue of overlap is. Can you think of a scenario like this? |
Absolutely. The use of a string-based protocol for Promises has lead to countless bugs. We've even recently discussed the issue arising from using dynamic import on a module that exports a
I'm not familiar enough with this particular design to comment on that. I was just providing you with background and a framework for you to use when making the decision. If the objects you expect to implement this protocol truly are single-purpose, a string-based protocol may be acceptable (though still probably not preferable). |
We discussed this at TC39. The notes from that discussion will be available at tc39/notes once approved. The general sentiment was that we should use strings here, because this is most likely the primary identity of a Calendar/TimeZone object, insofar as you aren't adding these calendar functions as you might add an iterator function to an existing type. One interesting option that was suggested was to "hide" the string fields inside a single symbol, such that you could call something like I think we should definitely move forward with the fields themselves being strings, which is also required for #291. The remaining open question then is whether we add a symbol to siphon the fields into another object, or whether we put them on the main object. |
Meeting Feb 20: Proceed with strings, without an extra symbol to encapsulate them. |
We discussed this a bit earlier, e.g., in #289 (comment) , but I wanted to open a separate issue to come to a conclusion on this one detail.
@sffc proposed using symbols stored as static properties, following the protocol proposal. I'd suggest using simple strings, as the protocols proposal has not been adopted, and strings are used for methods like this that already exist. I don't think we have to worry about namespace collisions here, which motivate some symbol use, and I think methods like
Temporal.Calendar.prototype.plus
are meaningful.The text was updated successfully, but these errors were encountered: