Add // @ts-ignore to typescript.d.ts AssertClause and AssertEntry entries for API backwards compatibility? #56954

JoshuaKGoldberg · 2024-01-04T22:14:03Z

🔍 Search Terms

api breaking change interface merge assert clause entry import attribute attributes

✅ Viability Checklist

This wouldn't be a breaking change in existing TypeScript/JavaScript code
This wouldn't change the runtime behavior of existing JavaScript code
This could be implemented without emitting different JS based on the types of the expressions
This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
This isn't a request to add a new utility type: https://github.com/microsoft/TypeScript/wiki/No-New-Utility-Types
This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

⭐ Suggestion

The AssertClause and AssertEntry interfaces in TypeScript 5.3.3 clash with their declarations in typescript-eslint - even after #56485. We don't see a way to resolve this for users. For the sake of backwards compatibility, could the types be given // @ts-ignores so that folks don't have to skipLibCheck?

    /** @deprecated */
+   // @ts-ignore
    interface AssertEntry extends ImportAttribute {
    }
    /** @deprecated */
+   // @ts-ignore
    interface AssertClause extends ImportAttributes {
    }

...and in general, whenever a type is @deprecated, could it be given a // @ts-ignore?

📃 Motivating Example

We haven't been able to figure out a way to fix this for users of typescript-eslint who want to keep skipLibCheck disabled.

I realize how gross it is to suggest // @ts-ignore inside TypeScript's types, but short of moving the types to DefinitelyTyped or patch-package, I'm out of ideas...

💻 Use Cases

typescript-eslint uses interface merging to ensure types for newly-added nodes are present regardless of TypeScript version:

// Workaround to support new TS version features for consumers on old TS versions
// Eg: https://github.com/typescript-eslint/typescript-eslint/issues/2388, https://github.com/typescript-eslint/typescript-eslint/issues/2784
declare module 'typescript' {
  /** @ts-ignore - added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
  export interface AssertClause extends ts.Node {}
  /** @ts-ignore - added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
  export interface AssertEntry extends ts.Node {}

Paraphrasing @bradzacher's typescript-eslint/typescript-eslint#8047 (comment):

TypeScript 4.5 - 5.2 declared AssertClause and AssertEntry with standard interfaces (interface AssertEntry extends Node { ... })
TypeScript 5.3 changed them to type aliases (type AssertEntry = ImportAttribute), which broke interface merging in typescript-eslint
Make AssertEntry and AssertClause interfaces #56485 -> TypeScript 5.3.3 changed them to interfaces (interface AssertEntry extends ImportAttribute { ... })

...but the 5.3.3 change didn't fix type checking on typescript.d.ts complaints for users. typescript-eslint/typescript-eslint#8047 shows project links & reports from users reporting they need to enable skipLibCheck:

node_modules/typescript/lib/typescript.d.ts:6021:15 - error TS2320: Interface 'AssertEntry' cannot simultaneously extend types 'ImportAttribute' and 'Node'.
  Named property 'kind' of types 'ImportAttribute' and 'Node' are not identical.

6021     interface AssertEntry extends ImportAttribute {
                   ~~~~~~~~~~~

The text was updated successfully, but these errors were encountered:

jakebailey · 2024-01-05T22:25:24Z

Can we just duplicate the declaration and deprecate one of them? Since they are structurally identical, they would be assignable to each other.

Not thrilled about having to special case this one thing in the dts bundler. (A regular comment like this will not be preserved and has no precedent.)

bradzacher · 2024-01-05T22:49:34Z

We preserve it in the dts by making it a block comment 😅

I'm more than happy if you guys wanna duplicate it and yeah it sucks that we need to keep this hack around.

Sadly though we'll be stuck with a hack until you guys remove it. We won't be able to remove ours until we bump our dep min past 5.3

jakebailey · 2024-01-05T23:49:20Z

If you get a chance, can you try the package built on #56967 to see if that "fixes" the problem?

JoshuaKGoldberg · 2024-01-06T22:40:22Z

👍 that fixes it! https://github.com/JoshuaKGoldberg/repros/tree/ts-eslint-tsc-assert-entry-clause-clash has a repro.

jakebailey · 2024-01-12T22:03:44Z

@JoshuaKGoldberg We realized in the design meeting that there is likely a much simpler fix; in your declarations, can you just specify the kind prop? For example:

  /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
  export interface AssertClause extends ts.Node {
    kind: ts.SyntaxKind.AssertClause;
  }
  /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
  export interface AssertEntry extends ts.Node {
    kind: ts.SyntaxKind.AssertEntry;
  }

https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgHIHsAmKDeBYAKGWOQGtRMAuZEAVwFsAjaAbkIF9DDRJZEUAgsggAPSCEwBnNFlyESZCtQCMbApwLdw0eEmQAhYWIgTpGbMhwatvXSkOjxU5EPxES5CSo5A

jakebailey · 2024-01-12T22:49:22Z

Hm, it seems like it doesn't work alone in your repro repo. I get fewer errors:


node_modules/typescript/lib/typescript.d.ts:6021:15 - error TS2320: Interface 'AssertEntry' cannot simultaneously extend types 'ImportAttribute' and 'Node'.
  Named property 'parent' of types 'ImportAttribute' and 'Node' are not identical.

6021     interface AssertEntry extends ImportAttribute {
                   ~~~~~~~~~~~

node_modules/typescript/lib/typescript.d.ts:6024:15 - error TS2320: Interface 'AssertClause' cannot simultaneously extend types 'ImportAttributes' and 'Node'.
  Named property 'parent' of types 'ImportAttributes' and 'Node' are not identical.

6024     interface AssertClause extends ImportAttributes {
                   ~~~~~~~~~~~~


Found 2 errors in the same file, starting at: node_modules/typescript/lib/typescript.d.ts:6021

However, having them do this does work:

    /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
    interface AssertClause extends ts.ImportAttributes {
    }
    /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
    interface AssertEntry extends ts.ImportAttribute {
    }

Or even:

    /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
    interface AssertClause extends ImportAttributes {
    }
    /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
    interface AssertEntry extends ImportAttribute {
    }

This would allow TS to not have to change anything. I would expect those to work because you're already having to declare these two new types for older TSs anyway?

jakebailey · 2024-01-12T22:58:55Z

I just tested the above two examples forcing TS 5.2, and they both compiled (well, almost; they complain about ts.JSDocParsingMode).

bradzacher · 2024-01-13T00:08:52Z

typescript-eslint/typescript-eslint#8181

I tried this on 5.2 and it errored.
Essentially I tested this by installing both in a test repo and then manually modifying the .d.ts files with the fix.

I'll check again in a bit - maybe I did something wrong in my test.

bradzacher · 2024-01-13T02:29:39Z

So as a base install with no changes:

$ yarn list @typescript-eslint/typescript-estree typescript
├─ @typescript-eslint/[email protected]
└─ [email protected]
✨  Done in 0.06s.

$ yarn tsc
✨  Done in 1.15s.

$ yarn list @typescript-eslint/typescript-estree typescript
├─ @typescript-eslint/[email protected]
└─ [email protected]
✨  Done in 0.06s.

$ yarn tsc
node_modules/typescript/lib/typescript.d.ts:6021:15 - error TS2320: Interface 'AssertEntry' cannot simultaneously extend types 'ImportAttribute' and 'Node'.
  Named property 'kind' of types 'ImportAttribute' and 'Node' are not identical.

6021     interface AssertEntry extends ImportAttribute {
                   ~~~~~~~~~~~

node_modules/typescript/lib/typescript.d.ts:6021:15 - error TS2320: Interface 'AssertEntry' cannot simultaneously extend types 'ImportAttribute' and 'Node'.
  Named property 'parent' of types 'ImportAttribute' and 'Node' are not identical.

6021     interface AssertEntry extends ImportAttribute {
                   ~~~~~~~~~~~

node_modules/typescript/lib/typescript.d.ts:6024:15 - error TS2320: Interface 'AssertClause' cannot simultaneously extend types 'ImportAttributes' and 'Node'.
  Named property 'kind' of types 'ImportAttributes' and 'Node' are not identical.

6024     interface AssertClause extends ImportAttributes {
                   ~~~~~~~~~~~~

node_modules/typescript/lib/typescript.d.ts:6024:15 - error TS2320: Interface 'AssertClause' cannot simultaneously extend types 'ImportAttributes' and 'Node'.
  Named property 'parent' of types 'ImportAttributes' and 'Node' are not identical.

6024     interface AssertClause extends ImportAttributes {
                   ~~~~~~~~~~~~


Found 4 errors in the same file, starting at: node_modules/typescript/lib/typescript.d.ts:6021

Then if I manually patch node_modules/@typescript-eslint/typescript-estree/dist/ts-estree/ts-nodes.d.ts

    /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
    interface AssertClause extends ts.ImportAttributes {
    }
    /** added in TS 4.5, deprecated and converted to a type-alias in TS 5.3 */
    interface AssertEntry extends ts.ImportAttribute {
    }

$ yarn list @typescript-eslint/typescript-estree typescript
├─ @typescript-eslint/[email protected] (PATCHED)
└─ [email protected]
✨  Done in 0.06s.

$ yarn tsc                                                 
✨  Done in 1.26s.

$ yarn list @typescript-eslint/typescript-estree typescript
├─ @typescript-eslint/[email protected] (PATCHED)
└─ [email protected]
✨  Done in 0.06s.

$ yarn tsc                                                 
✨  Done in 1.26s.

I'm so confused right now - I tested the above fix several times and it broke on TS5.2 with an error because it said that the interfaces had a different parent - that's why I ended up closing the PR and leaving that comment.

Well I guess I'll reopen and land that PR then 🤷

jakebailey · 2024-01-13T02:55:51Z

Well, I'm glad it works, anyway! That other linked PR didn't seem to allude to what broke, unless I misread something...

jakebailey · 2024-01-13T05:06:47Z

6.18.2-alpha.6 is working, so I'm going to close this.

…peScript#56954

RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 5, 2024

jakebailey mentioned this issue Jan 5, 2024

Copy and paste ImportAttribute/ImportAttributes to old names #56967

Closed

jakebailey mentioned this issue Jan 12, 2024

Update all dependencies microsoft/DefinitelyTyped-tools#906

Merged

jakebailey closed this as completed Jan 13, 2024

ryb73 added a commit to ryb73/treeqlite-js that referenced this issue Feb 27, 2024

Make it compile. Using old version of TS pending fix for microsoft/Ty…

8997269

…peScript#56954

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add // @ts-ignore to typescript.d.ts AssertClause and AssertEntry entries for API backwards compatibility? #56954

Add // @ts-ignore to typescript.d.ts AssertClause and AssertEntry entries for API backwards compatibility? #56954

JoshuaKGoldberg commented Jan 4, 2024 •

edited

Loading

jakebailey commented Jan 5, 2024 •

edited

Loading

bradzacher commented Jan 5, 2024

jakebailey commented Jan 5, 2024

JoshuaKGoldberg commented Jan 6, 2024

jakebailey commented Jan 12, 2024 •

edited

Loading

jakebailey commented Jan 12, 2024 •

edited

Loading

jakebailey commented Jan 12, 2024

bradzacher commented Jan 13, 2024

bradzacher commented Jan 13, 2024 •

edited

Loading

jakebailey commented Jan 13, 2024

jakebailey commented Jan 13, 2024

Add // @ts-ignore to typescript.d.ts AssertClause and AssertEntry entries for API backwards compatibility? #56954

Add // @ts-ignore to typescript.d.ts AssertClause and AssertEntry entries for API backwards compatibility? #56954

Comments

JoshuaKGoldberg commented Jan 4, 2024 • edited Loading

🔍 Search Terms

✅ Viability Checklist

⭐ Suggestion

📃 Motivating Example

💻 Use Cases

jakebailey commented Jan 5, 2024 • edited Loading

bradzacher commented Jan 5, 2024

jakebailey commented Jan 5, 2024

JoshuaKGoldberg commented Jan 6, 2024

jakebailey commented Jan 12, 2024 • edited Loading

jakebailey commented Jan 12, 2024 • edited Loading

jakebailey commented Jan 12, 2024

bradzacher commented Jan 13, 2024

bradzacher commented Jan 13, 2024 • edited Loading

jakebailey commented Jan 13, 2024

jakebailey commented Jan 13, 2024

JoshuaKGoldberg commented Jan 4, 2024 •

edited

Loading

jakebailey commented Jan 5, 2024 •

edited

Loading

jakebailey commented Jan 12, 2024 •

edited

Loading

jakebailey commented Jan 12, 2024 •

edited

Loading

bradzacher commented Jan 13, 2024 •

edited

Loading