Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TSError: Interface 'AssertClause' cannot simultaneously extend types 'ImportAttributes' and 'Node' #8047

Closed
4 tasks done
deot opened this issue Dec 11, 2023 · 13 comments · Fixed by #8181
Closed
4 tasks done
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working help wanted Extra attention is needed package: typescript-estree Issues related to @typescript-eslint/typescript-estree

Comments

@deot
Copy link

deot commented Dec 11, 2023

Before You File a Bug Report Please Confirm You Have Done The Following...

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have searched for related issues and found none that matched my issue.
  • I have read the FAQ and my problem is not listed.

Issue Description

When I use tsc, I get some errors when build.

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

It seems there are still errors after #7968 fix

#7968 (comment)

Reproduction Repository Link

https://stackblitz.com/edit/nhmdzj?file=index.ts&file=tsconfig.json

Repro Steps

  1. open link
  2. npm install
  3. npm run build

Screenshot of a comment on a GitHub issue showing an gif, Demo steps for repro

Versions

package version
@typescript-eslint/eslint-plugin 6.13.2
TypeScript 5.3.3
@deot deot added bug Something isn't working triage Waiting for maintainers to take a look labels Dec 11, 2023
Copy link

Uh oh! @deot, the image you shared is missing helpful alt text. Check your issue body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@bradzacher
Copy link
Member

See #7968 (comment)

Waiting for the TS fix to release with their next release. Until such time you will need to use skipLibCheck

@bradzacher bradzacher closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2023
@bradzacher bradzacher added working as intended Issues that are closed as they are working as intended external This issue is with another package, not typescript-eslint itself and removed triage Waiting for maintainers to take a look labels Dec 11, 2023
@threema-danilo
Copy link

@bradzacher are you sure this is fixed with TS 5.3.3?

With these versions:

$ npm list
[redacted]
├── @typescript-eslint/utils@6.14.0
└── typescript@5.3.3

...and this TS library definition:

$ grep -r "AssertEntry extends" node_modules/typescript/lib/typescript.d.ts
    interface AssertEntry extends ImportAttribute {

...I still get the TypeScript error.

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

@ashvinsrivatsa
Copy link

Yeah, this doesn't seem to be fixed. Here's a minimal repro:

#!/usr/bin/env sh
mkdir example
cd example
npm init --yes > /dev/null
npm install typescript@5.3.3 @typescript-eslint/utils@6.14.0 > /dev/null
echo '{ "compilerOptions": { "module": "NodeNext", "moduleResolution": "NodeNext" } }' > tsconfig.json
echo 'import "@typescript-eslint/utils";' > index.ts
npx tsc

npx tsc outputs the same compilation errors shown in #8047 (comment).

@JoshuaKGoldberg
Copy link
Member

👍 thanks for letting us know. Reopening and putting back into triage.

As a side curiosity, why are you folks all discovering this? It only happens if you don't have skipLibCheck enabled - is there a reason you don't have that flag?

@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for maintainers to take a look and removed working as intended Issues that are closed as they are working as intended labels Dec 16, 2023
@adidahiya
Copy link

Just encountered this issue as well while attempting to upgrade an ESLint plugin with custom rules to typescript-eslint 6.17.0 & typescript 5.3.3. We generally don't enable skipLibCheck in any project unless it's necessary as a hack to move forward, but we might do so in this case...

@octogonz
Copy link
Contributor

octogonz commented Jan 3, 2024

@JoshuaKGoldberg

As a side curiosity, why are you folks all discovering this? It only happens if you don't have skipLibCheck enabled - is there a reason you don't have that flag?

What does skipLibCheck do?

It tells the compiler to silently ignore any compile errors that are found in .d.ts files from external NPM packages.

Are compile errors bad?

Yes, that is why compilers check things and report errors. Compile errors can identify problems such as:

  • API calls that are simply going to fail because the type signatures don't match
  • Two incompatible versions of the same library getting imported simultaneously, due to problems such as incorrect peerDependencies or basic lack of coordination between library developers
  • Often, incompatible versions of the same library don't produce actual runtime errors, but instead may produce performance problems such as longer compile times or duplicate files bloating your runtime bundle

And one more, slightly harder-to-understand consequence: Suppose that consumer.ts imports a bunch of types from library.ts, and library.ts has numerous compile errors. If we ignore those errors, then the compiler doesn't have accurate declarations for library.ts; it must substitute approximations such as any for the expressions that it could not analyze. This compromises its ability to find mistakes in consumer.ts. And if consumer.ts exports inferred types, the generated .d.ts files may have inaccurate type declarations as well.

Then why would anyone ever want to disable compile checks?

Lots of people have been recommending skipLibCheck=true, mainly for two reasons:

  1. It avoids the effort required to fix the actual underlying problems, which is often difficult for beginners because it requires reading and understanding .d.ts files from someone else's NPM package, as well as understanding NPM dependency version management (which is a surprisingly advanced skill).

  2. People say tsc runs faster with skipLibCheck

Who suffers?

The skipLibCheck=true advice works pretty well for small projects, whose dependency conflicts often don't cause any runtime problems, and whose debugging scenarios are small enough to fit in your head. This is the same domain that deliberates whether to enforce type checking at all. Small projects comprise the vast majority of GitHub projects and blog authors.

On the other hand, large complex projects (especially in monorepos) greatly benefit from compiler checks. In this situation, people will often prefer to fight with occasional compile errors to avoid runtime errors that can be extremely frustrating to find and debug. And for the problem of 5 different versions of the same .d.ts files getting imported without anyone noticing, in a large high traffic code base, it saves sooo much time to fix that before it gets merged into main than after.

On the surface, then, this decision is a personal choice.

HOWEVER, if you make and publish an NPM package, and your NPM package was built with skipLibCheck=true, then it is likely that your NPM package may contain actual mistakes that cause compiler errors. In other words, nobody who consumes your NPM package will be able to choose skipLibCheck=true for themselves. TypeScript does not provide a way to skip checks for just one badly behaved library. Thus, there is a strong case that library maintainers should not ever be setting skipLibCheck=true.

@octogonz
Copy link
Contributor

octogonz commented Jan 3, 2024

Versions

package version
@typescript-eslint/eslint-plugin 6.13.2
TypeScript 5.3.3

This problem also repros with TypeScript 5.3.2.

Downgrading to TypeScript 5.2.2 avoids the error.

@bradzacher
Copy link
Member

If someone wants to try and solve this - happy to accept a PR. It's just a really, really hard problem, unfortunately. We are trying to support a range of TS versions and they add and remove node types which we depend on.

Unfortunately in 5.3 there was a change made which completely broke our existing style of handling the backwards-compatibility. We don't currently have a good solution to fix this.

Is skipLibCheck a good solution? No, obviously not - it's far from what we want consumers to do. Which is the entire reason this issue is open.

But without a concrete solution is the only one we've got right now.

More than happy to accept a PR if someone can figure out a solution. If this issue is important to you - consider being that champion.

@octogonz
Copy link
Contributor

octogonz commented Jan 3, 2024

@DanielRosenwasser @RyanCavanaugh

It's rather important for @typescript-eslint to be able to easily stay current with the latest TypeScript releases. Maybe a deeper solution is possible?

For example, perhaps the semantic declarations that @typescript-eslint depends on could provide a simplified adapter that is more stable across compiler releases? Similar to how the AST tree was abstracted.

Or perhaps skipLibCheck could be expanded so that it can skip an array of specific NPM package names (ironically typescript itself in this case), rather than requiring an all-or-nothing selection?

@bradzacher
Copy link
Member

For context. When TS shipped support for import assertions it added the new node types like

interface AssertEntry extends Node { ... }

For backwards-compatibility we added these nodes to our module type merge as before eg:

import * as ts from 'typescript';
declare module 'ts' {
  export interface AssertEntry extends Node {}
}

This is a bit of a hack but essentially in old versions of TS that don't support AssertEntry this creates an "empty" type that we can reference in our codebase and in new versions of TS that do support AssertEntry this is a no-op.
This hack relies on declaration merging.

In TS 5.3 support for the new and finalised syntax was introduced and the nodes were renamed. But rather than completely remove them instead they were converted to aliases to the new nodes eg

type AssertEntry = ImportAttribute;

This broke our types because you can't declaration merge an interface into a type. So microsoft/TypeScript#56485 was landed which converted them to interfaces to allow declaration merging. However this unfortunately doesn't completely fix the problem for us because we have two sets of incompatible types now.

// TS 4.5 - 5.2
interface AssertEntry extends Node { ... }
// TS 5.3.3+
interface AssertEntry extends ImportAttribute { ... }

This is an issue because there's no way we can define a no-op declaration that satisfies both definitions.

If we define this (which is what we currently do)

interface AssertEntry extends Node {}

Then TS>=5.3.3 will error because our no-op interface declares an extension that's different to the original.

If we instead define this:

interface AssertEntry extends ImportAttribute {}

Then TS<5.3 will error because our no-op interface declares an extension that's different to the original.

I don't know if there's a way to satisfy both constraints.
We have a @ts-ignore in our types - however TS reports an error within the typescript declaration files - which we obviously can't suppress.

So we're stuck between a rock and a hard place - we can't provide backwards compatibility support for our supported TS versions.

@bradzacher bradzacher added help wanted Extra attention is needed package: typescript-estree Issues related to @typescript-eslint/typescript-estree accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look external This issue is with another package, not typescript-eslint itself labels Jan 4, 2024
@bradzacher
Copy link
Member

If you'd like to try 6.18.2-alpha.6 (will be live in a few minutes) - it should have fixed this for v5.3.3+ and <=5.2.x

Note: v5.3.2 shall be forever broken as it uses a type alias - so please bump to 5.3.3.

@jakebailey
Copy link
Collaborator

I checked in DefinitelyTyped-tools, and I can confirm that 6.18.2-alpha.6 compiles without skipLibCheck. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue bug Something isn't working help wanted Extra attention is needed package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
8 participants