Typescript: `createPrivateIdentifier` API is weird

[DanielRosenwasser]

createPrivateIdentifier is a new part of our public surface API

https://github.com/Microsoft/TypeScript/blob/3b396e6124d6a0382c4ab5b2d846a9c4b65a4ea3/src/compiler/factoryPublic.ts#L220-L224

But it's weird because it takes a string whose contents need to start with # to get printed correctly. That seems strange because when its argument doesn't start with a #:

• it doesn't fail fast and
• it doesn't automatically prefix the contents with #

What should we do to make this better?

CC @rbuckton @joeywatts @mheiber

[mheiber]

Apologies for the oversight, glad you caught this issue @DanielRosenwasser !

Out of our two options:

• fail fast if contents don't start with #
• automatically prefix with #

TS team's experience better-informs what API will be most ergonomic, so please take the following with a grain of salt:

I have a slight preference for failing fast. My reason is that the following logs #foo, not "foo" (per spec, tested in Chrome as well):

new class { #foo = () => {}; constructor(){ console.log(this.#foo.name) } }

From the engine's point of view, the ,name is "#foo", so it seems to be consistent to require have text start with a # in createPrivateIdentifier(text)

[DanielRosenwasser]

Any thoughts on this @ajaff @dsherret? I'm inclined towards fail-fast, but I'd appreciate any input.

[dsherret]

It definitely would be nice for all PrivateIdentifier objects to be consistent.

I agree that # should be in the identifier name, but I don't think that's a reason for failing fast—it's just a reason to have # in the node's property's string value. I think if there's a way to not throw then that way should be favoured as it provides the least amount of user inconvenience and won't show up in a bug report one day in someone's repo. I'd vote for being forgiving and just add a # if the incoming string doesn't have one prefixed, but I don't feel strongly about it.

[sandersn]

Since the beta is coming to a close, let's fail fast for now and revisit the decision 3.8 ships. I'm not great at API design, but I know that if you start strict you can switch to less strict later.

[sandersn]

I have a quick PR up. Please take a look if you're interested.

[mheiber]

Thanks for the fix @sandersn and @DanielRosenwasser . I was too slow to comment before PR merge, but if I could go back in time I would :+1: :+1: fwiw