Is this a bug with lerna multi package project with TS4053 error?

[haha370104]

TypeScript Version: 4.0.3

Code

This is the repo of demo https://github.com/haha370104/lerna-typescript-error

These files just like this.

export const enum TestEnum {
    Test1 = '123123',
    Test2 = '12312312312',
}

export interface ITest {
    [TestEnum.Test1]: string;
    [TestEnum.Test2]: string;
}

export class A {
    getA(): ITest {
        return {
            [TestEnum.Test1]: '123',
            [TestEnum.Test2]: '123',
        };
    }
}

// packages/ts-error/src/index.ts

import {A} from './class';

export class B extends A {
    getA() { // TS4053 error
        return {
            ...super.getA(),
            a: '123',
        };
    }
}

Actual behavior:

The "getA" method of class B will cause an error as " error TS4053: Return type of public method from exported class has or is using name 'TestEnum' from external module "/Users/admin/lerna-test/packages/ts-error/src/class" but cannot be named". But if i move these two files to root director, the error will disappear.

I'm don't know why these two file will be recognized to be external module when they in the same lerna package. Also I'm not sure this is a ts issue or my mistake. But it has already cost days for me to search the answer but without result.

Playground Link:

Related Issues:

[MartinJohns]

Looks like a duplicate of #9944. I just searched for your error message (excluding the project relevant names) on Google.

[haha370104]

@MartinJohns Thanks for your reply. I've seen this issue yet, but unfortunately it doesn't look like a same issue

In #9944 case, I cannot even reproduce the compile error with typescript 4.0.0+，which I can reproduce the "mapping-Foo-Bar" example with typescript 2.0.0.

The point is, in my example repo, why the "class.ts" file is considered as a external module when it's in package/src path and be imported by "index.ts", even if they are in the same folder. But it will be fine if I move "index.ts" file and "class.ts" file to root directory?

[andrewbranch]

The point is, in my example repo, why the "class.ts" file is considered as a external module when it's in package/src path and be imported by "index.ts", even if they are in the same folder. But it will be fine if I move "index.ts" file and "class.ts" file to root directory?

Because when you move these files to the root directory, they’re no longer covered by your tsconfig’s include path, and we only issue this error when declaration is true (which is not the case when there’s no tsconfig including your files).

This does look like a declaration emitter bug, nothing to do with lerna or where you put your files. As a workaround, just give getA() an explicit return annotation (e.g. ITest & { a: string }).

[haha370104]

@andrewbranch Thanks for your reply. I am trying to develop a internal js library with lerna, so I have to set the declaration flag to true. If I have to give all getA method an return annotation, it will be a lot of another work I have to do.

So, is this a tsc bug instead of my mistake?

[andrewbranch]

I have to set the declaration flag to true

I understand, I was just answering your question about why the issue stopped when you moved your files.

So, is this a tsc bug instead of my mistake?

At a glance, it looks that way.

[weswigham]

Yes but no? I guess? So I looked at it, and it's because we're doggedly trying to preserve the computed names in the resulting object type of the spread - and the spread doesn't make an intersection, it makes a whole new object type. If you mouse over it in the editor, we print back the (rather unsatisfying) type

    a: string;
    123123: string;
    12312312312: string;
}

this is "as accurate as we could get" with the visibility rules at the position. Declaration emit is trying to preserve the string-enum-yness of the keys, and so is trying to write

{
    [TestEnum.Test1]: string,
    [TestEnum.Test2]: string,
    a: string,
};

but can't find TestEnum in scope. And since it's an expression position, it can't use an import type; so instead it issues the private name error saying an explicit type annotation is required.

Now, I say this, and this would indicate there's another way you could fix this in the code - adding an explicit import of TestEnum so it can be named, which should allow the computed names in the result, right? Ah... that's where we definitely have a bug. We actually have a bug in our name lookup for visibility checking. Bit of background: declarations within a module are typically represented by two symbols, a local symbol, and an export symbol. Local symbols go in the locals table, and export symbols in the export one. The local symbol is usually just a stub that points at the export symbol (these stubs are different from aliases). The issue arises when we're looking for the local symbol, but have to traverse the exports via aliases to find it. In such cases, we end up with the export symbol associated with the declaration, which isn't identical to the local symbol by any of the means we currently check, which in turn makes us think the symbol isn't visible, when, in fact, it is (because of the localSymbol/exportSymbol relationship). So let's say we fix that - you add a TestEnum import and everything's all right, right? Still no. Well, it is - you get a declaration file out and no error, but now you get an object type out that's the same as the quickinfo - no references to TestEnum at all. So what gives!?! We went through all that effort to check if it was available, and then went and didn't use it? Turns out we trackComputedName to issue visibility errors on the computed name like we're going to preserve the computed name, but then don't bother! This is a weird thing to do! Turns out, we're tracking the computed name, since we expect to need to make one in the result for symbol names, but we actually don't care about preserving computed-ness for stringy names, so since it's late bound it's getting the treatment intended for only symbol-type computed names.

All this to say: Yeah, there's a bug here, and this should be an easy fix (don't track computed name visibility for non-symbol names, #41806 is up), but it exposes some other broken stuff along the way to getting to that easy fix, which is maybe less straightforward to fix, but also probably less important.