| 01:03 | <shu> | thoughts on prohibiting computed property names in struct bodies? with structs positioned as "restricted classes that trade expressivity for performance and analyzability", prohibiting computed property names furthers the analyzability goal |
| 01:19 | <iain> | I would support it. If I'd had to guess without looking, I'd have assumed they were already prohibited. |
| 02:23 | <shu> | they are currently allowed, but not really intentionally |
| 05:10 | <rbuckton> | Would this then prohibit using symbol named properties like Symbol.dispose? If so, I am not in favor. |
| 06:14 | <shu> | the principle is statically analyzable name |
| 06:14 | <shu> | i wonder if there's a way to recover well-known symbol names and retaining analyzability |
| 06:17 | <ljharb> | then you also wouldn't be able to have a string property name? |
| 14:10 | <Ashley Claymore> | Was going to say: Well known `Symbol` fields are non-writable&non-configurable. so `[Symbol.iterator]` is statically known. expect Symbol itself could be replaced |
| 15:01 | <rbuckton> | Allowing [Symbol.dispose] but not [x] would be inconsistent and a source of confusion for users. |
| 15:03 | <rbuckton> | Why is this static analyzability necessary? The shape will be locked down when the definition is evaluated. I'd be fine with implementations having fast paths for statically analyzable definitions, but not with disallowing them entirely. |
| 15:06 | <rbuckton> | Static analysis for Symbol isn't necessarily reliable anyways given polyfills for new built in symbols, the fact you can redeclare Symbol in your module, and that you could have const x = Symbol.iterator. |
| 15:07 | <rbuckton> | I also have cases where I've used vm.Context in NodeJS to replace Symbol with a mock, which would likely violate static analysis. |
| 15:22 | <shu> | it's not necessary for the engine, but came as a request from tooling folks internally |
| 15:23 | <shu> | not sure i follow |
| 15:28 | <rbuckton> | I would be opposed to disallowing computed properties for the reasons stated: Disallowing entirely would break important use cases (e.g., Symbol.dispose, Symbol.iterator), and only allowing a restricted set of "statically analyzable" computed properties would be confusing, fragile, and break user expectations. |
| 15:29 | <shu> | a possibility is to disallow all computed property names for own fields, but allow it for instance fields (methods, getters), and static fields |
| 15:30 | <shu> | since:
|
| 16:43 | <littledan> | can you elaborate on static analyzability goals? Are we going for soundness or "works in practice"? |
| 16:52 | <rbuckton> | Disallowing for own fields but allowing for methods is just another source of confusion for users. |
| 16:53 | <rbuckton> |
Not all use cases today are for symbols on the prototype. NodeJS internals are rife with symbols defined on instance objects. |
| 16:53 | <rbuckton> | And none of those protocols require those symbols be on the prototype. |
| 16:57 | <shu> | and is it important to support those use cases with structs? |
| 16:58 | <shu> | "works in practice" for sure, but by construction than by, say, linting |
| 16:59 | <rbuckton> | I believe it is, yes. |
| 16:59 | <shu> | can you show me an example? |
| 17:00 | <rbuckton> | Shortly, yes. I'm about to step into a meeting. |
| 17:00 | <iain> | I seem to recall that we were talking about source location as a mechanism for solving the coordination problem. Has that changed? If computed properties are allowed, then the same source location can define multiple distinct types. |
| 17:01 | <shu> | i've been talking about unshared structs |
| 17:01 | <shu> | but yeah that's a good point for the shared ones |
| 17:03 | <rbuckton> | If they didn't point to the same properties, they wouldn't match, much like how if you had two references to the same source location with different field layouts wouldn't match (say, due to a new version of the file being loaded in a worker) |
| 17:04 | <shu> | the use cases all revolve around symbols. i'd like to solve that more directly |
| 17:05 | <shu> | being declarative is a good goal; i think there's a world of difference between supporting Symbol.iterator, which is important, and supporting arbitrary computation for field names |
| 17:05 | <rbuckton> | NodeJS might want to take advantage of the fixed field layout and one shot initialization of structs. |
| 17:06 | <rbuckton> | There are many across the codebase. You could imagine examples like this being converted to a struct and needing computed property names to declare fields: https://github.com/nodejs/node/blob/01dd7c4f6450ac5f092f98a5a85f00d0a7f489b2/lib/wasi.js#L113-L118 |
| 17:08 | <shu> | okay i see |
| 17:08 | <rbuckton> | Just search https://github.com/search?q=repo%3Anodejs%2Fnode+%22Symbol%28%22+language%3AJavaScript&type=code and look at the references for any symbol defined as const kName = Symbol(...) |
| 17:29 | <rbuckton> | And there are numerous cases of user defined symbols all over GitHub, though its difficult to craft a search query for computed property usages themselves |
| 17:34 | <rbuckton> | Though searching for computed property names isn't even useful, since any class currently using this[x] = v in the constructor instead of fields that wanted to convert to struct would, by necessity, need to convert to computed property names to declare the fields. |
| 17:37 | <rbuckton> | some of those symbols are used instead of # private, but many others are used as a way for different objects in the same package to communicate with each other, or as a poor man's protected. They are also often used as user-defined protocols, such as util.inspect.custom in NodeJS |
| 17:39 | <rbuckton> | I think there are far too many use cases for user-defined symbols to exclude them. I think possibly the only corner case I'd disallow is local symbol-named properties in a shared struct, while still allowing built-ins and Symbol.for |
| 17:40 | <rbuckton> | (and Symbol.for could conceivably be handled by associating the field name in the struct with the symbol description passed to Symbol.for rather than the symbol itself) |
| 17:47 | <rbuckton> | And inspect.custom is used as an instance field in a number of cases (https://github.com/search?q=%2Fthis%5C%5B%28%5Cw%2B%5C.%29%3Finspect%5C.custom%5D+%3D%2F+language%3AJavaScript&type=code) |
| 17:50 | <shu> | well, not everything needs to be converted to structs |
| 17:50 | <shu> | i am pretty convinced of the symbol use case, especially the well-known ones |
| 17:50 | <rbuckton> | Yes, but some things will be and this change would be a significant barrier to that |
| 17:51 | <rbuckton> | I also use user-defined symbols for protocols in some of my code for things that I would like to convert to struct or shared struct, and this change would block those use cases. |
| 17:52 | <shu> | well hold on |
| 17:52 | <shu> | you're wholly against any kind of symbol carve out? |
| 17:52 | <shu> | it's either arbitrary computed property names or nothing? |
| 17:52 | <rbuckton> | I already stated that. A built-in Symbol carve out is fragile and unreliable. |
| 17:53 | <shu> | i see the developer surprise point but i don't see why it's unreliable |
| 17:53 | <rbuckton> | Even NodeJS would run afoul of a built-in Symbol carveout due to the common practice of saving off primordials |
| 17:56 | <rbuckton> | https://github.com/nodejs/node/blob/01dd7c4f6450ac5f092f98a5a85f00d0a7f489b2/lib/internal/per_context/primordials.js#L300 |
| 17:56 | <shu> | yeaaah, i see |
| 17:57 | <rbuckton> | So a Symbol carve-out would preclude a common security pattern, unless you are also doing some kind of whole-program static analysis |
| 17:57 | <littledan> | do you mean you want soundness? |
| 17:58 | <littledan> | "by construction" sounds sound |
| 17:58 | <shu> | i mean i don't care about soundness |
| 17:58 | <shu> | but it's sounding more and more like there's no good way to allow for the Symbol use case, which is important, without allowing everything |
| 17:59 | <littledan> | OK so one simple solution (aside from what's discussed above) is for the language to allow random computed properties, and the static analysis can reject things which are not Symbol.* and assume that everything which is Symbol.* is what you're guessing |
| 17:59 | <shu> | there's what decorators do with @MemberExpression but even there you can escape it via @ParenthesizedExpression |
| 17:59 | <littledan> | yeah but your analysis can just reject those things |
| 17:59 | <littledan> | even if the language accepts them |
| 18:00 | <littledan> | (or the analysis just gives up and accept it but refuses to analyze) |
| 18:00 | <shu> | oh you mean the compiler? right |
| 18:00 | <shu> | i'm coming around to that's the right approach -- because no more runtime guarantees are needed. this is just about making assumptions at parse time of the declaration |
| 18:01 | <ljharb> | oh i guess nvm, it's only symbols since you don't need brackets for strings |
| 18:01 | <shu> | right, both strings and indexed strings already have a bare form in the syntax |
| 18:01 | <shu> | indexed strings meaning the numbers |
| 18:01 | <shu> | of course you could do ["foo"] but why |
| 18:04 | <iain> | To be clear: we still intend to prohibit computed properties for shared structs, right? |
| 18:07 | <shu> | syntactically? is there a need to? you want to do the correlation at parsing time instead of struct-definition-evaluation time? |
| 18:07 | <shu> | at runtime, Symbol properties are outright prohibited because they can't be shared (there can be a carveout for Symbol.for symbols but those things are kind of useless and you should use a string) |
| 18:11 | <ljharb> | so you can't have an iterable struct? |
| 18:11 | <iain> | I see very little value in non-Symbol computed properties. It seems like it makes correlation strictly more difficult than if we prohibited it. |
| 18:11 | <shu> | i'm certainly fine with prohibiting it |
| 18:12 | <iain> | I'm not going to die on this hill or anything, it just seems like we just make things complicated for ourselves for no reason |
| 18:12 | <shu> | unfortunately no, not without a wrapper object, because Symbol.iterator isn't a shared thing currently. though i suppose it's unobservable since Symbols can't be structure cloned so the identity is not observable across threads, so maybe we can retcon it to be a shared thing |
| 18:13 | <ljharb> | well-known symbols are cross-realm, aren't they? |
| 18:13 | <shu> | but that doesn't say anything about cross-agent |
| 18:13 | <ljharb> | ah |
| 18:13 | <shu> | i think we can retcon them but we'd have to decide |
| 18:13 | <shu> | and that does reopen the computed property name question for shared structs as well |
| 18:14 | <shu> | well, at least that'd mean there's a reason to allow them |
| 18:15 | <shu> | the retconning may be possible for well-known symbols easily enough but for user symbols and polyfills it's not clear |
| 18:15 | <shu> | i think it's mainly an implementation burden rather than a language one |
| 18:16 | <shu> | though, an implementation has to solve sharing strings, so sharing symbols shouldn't be too hard |
| 18:17 | <shu> | but for polyfills, how does worker A coordinate its polyfill of Symbol.someNewProtocolSymbol with worker B's polyfill of it, so that there's one copy that both threads know to use? |
| 18:17 | <shu> | we can say, that case isn't supported out of the box, sorry, you have to manually coordinate |
| 18:17 | <shu> | or not even polyfills, normal library coordination, i guess |
| 18:17 | <nicolo-ribaudo> | but for polyfills, how does worker A coordinate its polyfill of |
| 18:19 | <shu> | what are registered symbols? oh Symbol.for? |
| 18:20 | <shu> | yes, that's a possibility, if we retcon symbols to be inherently shareable things + make Symbol.for registry process-wide |
| 18:20 | <shu> | and that might be interesting in giving Symbol.for a reason for its pitiful existence |
| 18:23 | <rbuckton> | TypeScript used to only support Symbol.x, but we introduced a late binding mechanism during check to resolve user-defined symbols and Symbol.for(), which requires whole-program analysis. |
| 19:16 | <rbuckton> | Polyfills for built-in symbol should use Symbol.for(), not Symbol(). |
| 19:19 | <shu> | oh okay |
| 19:19 | <shu> | didn't know that was already best practice |
| 19:21 | <Ashley Claymore> | part of me wonders if it would be web compat to say that all the well knows are registered symbols. So `Symbol.iterator === `Symbol.for("com.ecma.262.symbol.iterator")` |
| 19:22 | <shu> | dang |
| 21:31 | <ljharb> | oof, i doubt it would be |
| 21:31 | <ljharb> | i have tests in multiple packages that would fail if Symbol.keyFor started returning strings for well known symbols |
| 21:34 | <nicolo-ribaudo> | A string with a [[Is262WKS]] ("is 262 well-known symbol") that makes `== undefined` return true on it |
| 21:37 | <shu> | sick |