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 wildcard export #8331

Open
astegmaier opened this issue Jul 21, 2022 · 1 comment 路 May be fixed by #8333

Comments

@astegmaier
Copy link
Contributor

astegmaier commented Jul 21, 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 a top-level export and the contents of a module that's exported with a wildcard export (e.g. export * from "foo).

Interestingly, the bug will appear or not depending on the order of the top-level exports (see below)

馃帥 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

export const nameConflict = { messageFromIndex: "this instance of nameConflict is from index.ts" };
export * from "./name-conflict"; // Note: this comes _after_ the top-level export above.

src/name-conflict.ts

export const nameConflict = { messageFromNameConflict: "this instance of nameConflict is from name-conflict.ts" };

馃槸 Current Behavior

When you run the parcel build, parcel generates the following index.d.ts file:

types.d.ts

export const nameConflict: {
    messageFromNameConflict: string;
};
declare const _nameConflict1: {
    messageFromIndex: string;
};

However, when you consume the library in a different project, the nameConflict export at runtime will be the top-level export from index.ts:

import { nameConflict } from "test-parcel-library";
// @ts-expect-error: the d.ts file generated by parcel is incorrect
console.log(nameConflict.messageFromIndex); // prints "this instance of nameConflict is from index.ts"

馃 Expected Behavior

The index.d.ts file generated by parcel should be:

declare const _nameConflict1: {
    messageFromNameConflict: string;
};
export const nameConflict: {
    messageFromIndex: string;
};

Interestingly, if you switch the order of the exports in index.ts, you'll get this correct output:

index.ts

export * from "./name-conflict"; // Note: this comes _before_ the top-level export below, which fixes the problem.
export const nameConflict = { messageFromIndex: "this instance of nameConflict is from index.ts" };

If you build the same project with tsc directly, the .d.ts files generated will correctly describe the package, regardless of the order of the exports (although it looks slightly different from what you'd expect from parcel because tsc can't bundle the .d.ts files for commonjs projects).

馃拋 Possible Solution

In TSModuleGraph.propegate we first collect all the top-level export names in an array with a call to getAllExports, and iterate through them:

let exportedNames = new Map<string, TSModule>();
for (let e of this.getAllExports()) {
this.markUsed(e.module, e.imported, context);
e.module.names.set(e.imported, e.name);
names[e.name] = 1;
exportedNames.set(e.name, e.module);
}

In the example above (where the bug reproduces), this array will have these (simplified) contents, in this order:

[ 
   { name: "nameConflict", module: <TSModule for 'index.ts"> }
   { name: "nameConflict", module: <TSModule for 'nameConflict.ts"> }
]

The call to exportedNames.set(e.name, e.module) will thus happen twice with the same name, over-writing the previous value. At the end of the loop, exportedNames will be:

{ "nameConflict":  <TSModule for 'nameConflict.ts"> }

Further down, when we assign unique names, we do this check:

for (let m of this.modules.values()) {
for (let [orig, name] of m.names) {
if (exportedNames.has(name) && exportedNames.get(name) === m) {
continue;
}

exportedNames.get(name) === m will be true when we're passing through nameConflict.ts, so we skip renaming it. When we pass through index.ts, it will be false, so we'll proceed to assign a disambiguator to it.

If you flip the order of the exports in index.ts that will change the contents of exportedNames, and you'll get the opposite (correct) behavior - i.e. the export from nameConflict.ts gets the disambiguator, and the top-level export from index.ts does not.

One solution would be to check when we are about to over-write entries to exportedNames and somehow find a way to deterministically prefer top-level exports over implicit exports that happen through a wildcard.

馃敠 Context

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

馃捇 Code Sample

You can see a reproduction of the problem in this github 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
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs.

@github-actions github-actions bot added the Stale Inactive issues label Jan 18, 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