Skip to content

Isolated declarations uses type information when emitting computed object keys Β #58533

Open

Description

πŸ”Ž Search Terms

  • isolated declarations
  • computed keys
  • object literals

πŸ•— Version & Regression Information

5.5.0-dev.20240514

⏯ Playground Link

https://www.typescriptlang.org/play/?isolatedDeclarations=true&ts=5.5.0-dev.20240514#code/KYDwDg9gTgLgBAYwgOwM7wNLAJ5wLxwBEMAFsFtoQNwBQNAJsAgDYCGUwcA5sxAEatmcAN4044uKEixEKdHADivAcwr4ipYEv6CKhMRKnR4SNPACqyAJYBHAK7AAytgC2fCMwBccO9fudUV3dmWglJcGNZMzgAORRLWwdnNw9vQJSQmgBfOiMZU3kAQXVRMIBtCgBdbxgoBwAaA3Ey5OCAOhgIR1qrZC4AFVYuargAM0FUYEbyhP9WjxHx5knpiTLtFSrvJZXsmiA

πŸ’» Code

export const Key = "theKey";

declare global {
    export const GlobalKey = "theGlobalKey"
    export const UniqueSymbol: unique symbol;
    export const NonUniqueSymbol: symbol;
}

export const A = {
    [Key]: true,
    [Symbol.toStringTag]: false,
    [UniqueSymbol]: false,
    [GlobalKey]: false,
}

πŸ™ Actual behavior

TypeScript is emitting the computed property keys with type information that can not be determined without a type checker.

export declare const Key = "theKey";
declare global {
    export const GlobalKey = "theGlobalKey";
    export const UniqueSymbol: unique symbol;
    export const NonUniqueSymbol: symbol;
}
export declare const A: {
    theKey: boolean;
    [Symbol.toStringTag]: boolean;
    [UniqueSymbol]: boolean;
    theGlobalKey: boolean;
};

πŸ™‚ Expected behavior

export declare const Key = "theKey";
declare global {
    export const GlobalKey = "theGlobalKey";
    export const UniqueSymbol: unique symbol;
    export const NonUniqueSymbol: symbol;
}
export declare const A: {
    [Key]: true,
    [Symbol.toStringTag]: false,
    [UniqueSymbol]: false,
    [GlobalKey]: false,
};

Additional information about the issue

No response

Activity

  1. weswigham commented on May 14, 2024

    @weswigham
    Member

    FWIW, I don't think computed property names are compatible with isolatedDeclarations at 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, expr must 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.

  2. lucacasonato commented on May 14, 2024

    @lucacasonato
    Author

    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+1EA

    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.

  3. weswigham commented on May 14, 2024

    @weswigham
    Member

    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).

  4. lucacasonato commented on May 14, 2024

    @lucacasonato
    Author

    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.

  5. lucacasonato commented on May 16, 2024

    @lucacasonato
    Author

    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.

  6. weswigham commented on May 23, 2024

    @weswigham
    Member

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Domain: Declaration EmitThe issue relates to the emission of d.ts filesDomain: flag: isolatedDeclarationsRelated to the --isolatedDeclarations compiler flagPossible ImprovementThe current behavior isn't wrong, but it's possible to see that it might be better in some cases

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions