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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

@parcel/transformer-typescript-types generates incorrect .d.ts file when there is a naming conflict with a namespace import #8360

Open
astegmaier opened this issue Aug 2, 2022 · 0 comments 路 May be fixed by #8366

Comments

@astegmaier
Copy link
Contributor

astegmaier commented Aug 2, 2022

馃悰 bug report

When bundling a library with parcel and relying on @parcel/transformer-typescript-types to generate a bundled .d.ts file, you'll get incorrect output if there is a naming conflict between things that are exported by a module consumed as a namespace import (e.g. import * as MyNamespace from "./foo.

馃帥 Configuration (.babelrc, package.json, cli command)

package.json

{
    "name": "test-parcel-library",
    "main": "dist/index.js",
    "types": "dist/types.d.ts",
    "scripts": {
        "build": "parcel build src/index.ts"
    }
    ...
}

src/index.ts

import * as NamespaceImport from "./other1";
export const consumer1: typeof NamespaceImport.nameConflict = { messageFromOther1: "foo" }
export const consumer2: typeof NamespaceImport.notAConflict = { messageFromOther2: "foo" }

src/other1.ts

export { nameConflict as notAConflict } from "./other2";
export const nameConflict = { messageFromOther1: "foo" };

src/other2.ts

export const nameConflict = { messageFromOther2: "foo" };

馃槸 Current Behavior

When you build this project with parcel, and try to consume it, the types don't describe the runtime behavior. If you build the same library with tsc, you get correct types.

import * as Parcel from "test-library-parcel";
import * as Tsc from "test-library-tsc"; // This library has the same source code, but it's built with tsc.

it("Library built with tsc has types match runtime behavior", () => {
    expect(Tsc.consumer1.messageFromOther1).toBe("foo"); // Type: { messageFromOther1: string; }
    expect(Tsc.consumer2.messageFromOther2).toBe("foo"); // Type: { messageFromOther2: string; }
});

it("Library built with parcel has types that do not match runtime behavior", () => {
    // @ts-expect-error
    expect(Parcel.consumer1.messageFromOther1).toBe("foo") // Type: { messageFromOther2: string; }
    expect(Parcel.consumer2.messageFromOther2).toBe("foo") // Type: any
});

Here is the index.d.ts file that parcel currently generates:

types.d.ts

declare const nameConflict: {
    messageFromOther2: string;
};
export const consumer1: typeof nameConflict;
export const consumer2: typeof NamespaceImport.notAConflict;

馃 Expected Behavior

The types should correctly describe the runtime behavior. This would be index.d.ts I would expect, were this to be fixed:

declare const nameConflict: {
    messageFromOther1: string;
};
declare const _nameConflict1: {
    messageFromOther2: string;
};
export const consumer1: typeof nameConflict;
export const consumer2: typeof _nameConflict1;

馃拋 Possible Solution

I think at least part of the problem lies in the way that have implemented resolveExport:

resolveExport(
module: TSModule,
name: string,
): ?{|imported: string, module: TSModule, name: string|} {
for (let e of module.exports) {
if (e.name === name) {
return this.getExport(module, e);
} else if (e.specifier) {
const m = this.resolveExport(
nullthrows(this.getModule(e.specifier)),
name,
);
if (m) {
return m;
}
}
}
}

When we are resolving the namespaced names (e.g. NamespacedImport.nameConflict), we'll iterate through all the exports other1.ts, in order. The first export (export { nameConflict as notAConflict };) hit the recursive condition on line 146, which causes it to go looking for nameConflict in other2.ts, which it finds and returns. This is incorrect because we didn't consider that might rename the export in the process of re-exporting.

There might be more layers to the onion, too.

馃敠 Context

I was working on a PR add support for namespace exports to @parcel/transformer-typescript-types, and I ran across this issue in the process.

馃捇 Code Sample

You can run the above examples in the namespace-imports branch of this repo

馃實 Your Environment

Software Version(s)
Parcel 2.6.2
Node 16.15.1
Yarn 1.22.19
Operating System Windows 11 Version 10.0.22000 Build 22000
@github-actions github-actions bot added the Stale Inactive issues label Feb 1, 2023
@parcel-bundler parcel-bundler deleted a comment from github-actions bot Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants