You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Isolated declarations uses type information when emitting computed object keys Β #58533
FWIW, I don't think computed property names are compatible with isolatedDeclarationsat a fundamental level - wherever we land on the fix is going to be a compromise. Computed names are, by their nature, a window from the type structure world into nonlocal expression checking space. I'm willing to say "we do the right thing when all the information is in the file" for the sake of convenience, since when the information isn't in the file, even a simple [key]: whatever is completely unsafe and not transformable into a type that's guaranteed to work the same way as the input implies. A export const a: {[expr]: number} does not have the same behavior as a export const a = {[expr]: 1}, and the differences all come down to the type of expr. As an expression, we allow expr to be string, any - any number of non-literal things. And depending on context, those are either elided entirely, transformed into index signatures, or maybe even splat out into multiple optional properties. In a type, exprmust typecheck as a property name literal type to be valid (not a union thereof, not a PropertyKey extending param, exactly a literal type). Otherwise we issue a A computed property name in a type literal must refer to an expression whose type is a literal type or a 'unique symbol' type. error when we read the type back in.
When it comes down to it, there's no simple rule to transform from an expression computed name to a type one that actually works (without full potentially nonlocal context). Any "rule" we apply is heuristic, at best. :S
Anyways, also ref #58490 which is on the same subject.
This issue was not about changing the checker, just about changing the emit behaviour specifically for the allowed types (string literals, number literals, or unique symbol), to retain the original expression rather than replacing it with the type value.
There is no semantic issue here as far as I can tell, it's just something that is needed to achieve token level equality of the output.
There is no semantic issue here as far as I can tell, it's just something that is needed to achieve token level equality of the output.
ish? The ID error can't be correctly emitted without full type information (see #58490), so you can't know if it's even safe to copy an expression computed name to a type computed name without doing whole-program typechecking. So while ID does have an error for this, it fails to emit it under the transpileDeclaration API (and just allows a potentially very wrong transform).
Yeah but that's already the case for other transforms, no? My understanding was that isolated declarations always operates on the assumption that the code is valid and does type check. Maybe I am misunderstanding this.
I retract my previous comment. I don't think it is reasonable for the emitter to have to make the assumption that the ID checker passed. Doing so would make it impossible to analyze without TSC's ID checker whether an arbitrary given file is eligible for ID emit in the first place, which makes the performance gains quite useless. You would still have to check all files anyway to determine whether you can ID emit them before emit.
These are all errors for now, but as we allow some of these forms as special cases, we do want to think about possibly aligning the emit output where possible, so this'll track that.
Activity
weswigham commentedon May 14, 2024
FWIW, I don't think computed property names are compatible with
isolatedDeclarationsat a fundamental level - wherever we land on the fix is going to be a compromise. Computed names are, by their nature, a window from the type structure world into nonlocal expression checking space. I'm willing to say "we do the right thing when all the information is in the file" for the sake of convenience, since when the information isn't in the file, even a simple[key]: whateveris completely unsafe and not transformable into a type that's guaranteed to work the same way as the input implies. Aexport const a: {[expr]: number}does not have the same behavior as aexport const a = {[expr]: 1}, and the differences all come down to the type ofexpr. As an expression, we allowexprto bestring,any- any number of non-literal things. And depending on context, those are either elided entirely, transformed into index signatures, or maybe even splat out into multiple optional properties. In a type,exprmust typecheck as a property name literal type to be valid (not a union thereof, not aPropertyKeyextending param, exactly a literal type). Otherwise we issue aA computed property name in a type literal must refer to an expression whose type is a literal type or a 'unique symbol' type.error when we read the type back in.When it comes down to it, there's no simple rule to transform from an expression computed name to a type one that actually works (without full potentially nonlocal context). Any "rule" we apply is heuristic, at best. :S
Anyways, also ref #58490 which is on the same subject.
lucacasonato commentedon May 14, 2024
Wesley Wigham (@weswigham) I agree with you - however I think ID already handles this on a checker level, because the checker disallows any types in computed key expressions that are not string literals, number literals, or
unique symbol- types that would result in type emitted output that is not a property member, but a index member or similar are already forbidden (number,string,symbol). See https://www.typescriptlang.org/play/?isolatedDeclarations=true&ts=5.5.0-dev.20240514#code/KYDwDg9gTgLgBAYwgOwM7wNLAJ4EYBcc6UAlsgOYDcAUKJLIiunFtgEyHICuAtgEbAoNOtHhI0mHAGZCqbPwgAbarXCjGEuAEE4AXjgBvanBNwA2q1wBdQjChdgAGmOmLONjbh2Hz0+dZSnt5O1AC+1EAThis issue was not about changing the checker, just about changing the emit behaviour specifically for the allowed types (string literals, number literals, or
unique symbol), to retain the original expression rather than replacing it with the type value.There is no semantic issue here as far as I can tell, it's just something that is needed to achieve token level equality of the output.
weswigham commentedon May 14, 2024
ish? The ID error can't be correctly emitted without full type information (see #58490), so you can't know if it's even safe to copy an expression computed name to a type computed name without doing whole-program typechecking. So while ID does have an error for this, it fails to emit it under the
transpileDeclarationAPI (and just allows a potentially very wrong transform).lucacasonato commentedon May 14, 2024
Yeah but that's already the case for other transforms, no? My understanding was that isolated declarations always operates on the assumption that the code is valid and does type check. Maybe I am misunderstanding this.
lucacasonato commentedon May 16, 2024
I retract my previous comment. I don't think it is reasonable for the emitter to have to make the assumption that the ID checker passed. Doing so would make it impossible to analyze without TSC's ID checker whether an arbitrary given file is eligible for ID emit in the first place, which makes the performance gains quite useless. You would still have to check all files anyway to determine whether you can ID emit them before emit.
weswigham commentedon May 23, 2024
These are all errors for now, but as we allow some of these forms as special cases, we do want to think about possibly aligning the emit output where possible, so this'll track that.