microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.71k stars 12.45k forks source link

A function that returns a class with a property getter is unusable via a declaration file (TS2611) #54879

Open kring opened 1 year ago

kring commented 1 year ago

Bug Report

🔎 Search Terms

TS2611, declaration file, property getter

🕗 Version & Regression Information

⏯ Playground Link

This can't be completely reproduced in the Playground because it requires two separate projects. One to produce a .d.ts and another to attempt (and fail) to use it.

💻 Code

Complete and minimal code sample here: https://github.com/kring/ts2611-in-decl-files You can reproduce it by cloning that repo and running npm install && npm run build.

To recreate it manually, define two projects, a and b, and use project references to link them up. Project a exports a class which is created by deriving from a class returned by a function, like this:

function functionReturningClass() {
  return class ClassInsideFunction {
    get r() {
      return 7;
    }
  };
}

class Something extends functionReturningClass() {
  override get r() {
    return 8;
  }
}

export default Something;

(The above is in https://github.com/kring/ts2611-in-decl-files/blob/main/a/something.ts)

Now if we add another module that uses this to the same project, then it works great:

import Something from "./something.js";

const d = new Something();
console.log(d.r);

(the above is in https://github.com/kring/ts2611-in-decl-files/blob/main/a/use-from-a.ts)

However, if we add identical code to another project that references this one, like this, then it no longer works anymore:

import Something from "../a/something.js";

const d = new Something();
console.log(d.r);

(the above is from https://github.com/kring/ts2611-in-decl-files/blob/main/b/use-from-b.ts)

🙁 Actual behavior

TypeScript reports the following:

out/a/something.d.ts:7:9 - error TS2611: 'r' is defined as a property in class '{ readonly r: number; }', but is overridden here in 'Something' as an accessor.

7     get r(): number;
          ~

This happens because the generated .d.ts does in fact mix properties and accessors:

declare const Something_base: {
    new (): {
        readonly r: number;
    };
};
declare class Something extends Something_base {
    get r(): number;
}
export default Something;

As far as I can tell, this makes the generated .d.ts file unusable.

🙂 Expected behavior

I expected no compiler error, probably because the TS compiler emitted an accessor in Something_base rather than a property. Or maybe because it didn't get hung on the mismatch in this scenario?

This was also previously observed here: https://github.com/microsoft/TypeScript/issues/41347#issuecomment-919256478

Thank you for taking a look!

athewsey commented 9 months ago

Believe I just came across this same issue while drafting amazon-textract-response-parser@0.4.0-alpha.1 (available on NPM) - on the WithWords and WithContent mixins.

I see same behaviour where if I override mixin-defined property getters (like here) the project builds just fine, but trying to use the library from another TypeScript project raises errors because of the inconsistent generated type declarations.

Observed with both TS v4.6.3 and v5.3.3 so far.

It looks like the linked PR may have stalled a bit... If anybody has user-side workarounds I'd be interested to hear them!

ClementValot commented 7 months ago

I have worked on a minimal-ish reproduction (couldn't get a simple one, so worked backwards from an excerpt of my code and removed stuff until I had a minimal configuration triggering the error), and only now saw that this issue was rescheduled 🤞

In case it helps: https://github.com/ClementValot/TS2611-repro (I'll try and clean it up even further)

KonnorRogers commented 4 months ago

I also hit this issue today. Here's my minimal reproduction:

https://tsplay.dev/mA0RkN

Andarist commented 4 months ago

@KonnorRogers I think that your issue is different. It's related to intersections and you can already observe the problem when type-checking the source file. The issue here manifests itself in the emitted declaration file. The linked PR also focuses on type serialization so it wouldn't fix your issue. I'd say that you should report a dedicated issue about your problem.

KonnorRogers commented 4 months ago

@Andarist will do.

technically my issue manifests itself in the "2 project" problem, because that mixin is coming from a a JSDOC based library I maintain that adds a getter in a mixin, and then outputs a .d.ts file with an accessor instead of a getter, but if you think a separate issue makes sense, I'll file one

Andarist commented 4 months ago

Can't say how close that setup is to this issue here but the playground that you shared is sufficiently different from this issue here, I think.