Resolve export default expression name collision

[Atrue]

Fixes #116

Issue

exporting default unnamed expression bundles the same name

// file ./number.ts
export default 0;

// file ./object.ts
export default {
  type: 'object',
};

// file ./string.ts
export default '';

// file ./input.ts
export { default as number } from './number';
export { default as object } from './object';
export { default as string } from './string';

result:

declare const _default: 0;
declare const _default: "";
declare const _default: {
	type: string;
};

export {
  _default as number,
  _default as object,
  _default as string,
};

See 3be37c9 for reference

Suggestion

Using a nameCollision map to detect and rename variable collisions.

declare const _default: 0;
declare const _default$2: "";
declare const _default$3: {
	type: string;
};

export {
  _default as number,
  _default$2 as string,
  _default$3 as object,
};

See 6c033c0 for reference

Restrictions

There are the same restrictions as with the current unnamed class or function statements behaviour export default class {}.
It can not be used as reference for other declarations:

import something from './file-with-export-default`;

export type T = typeof something; // do not work (even with the unnamed class or function)

Similar issues

It looks like it also fixes this #116

[timocov]

Hi @Atrue,

I'm sorry for late reply. Thanks for your contribution to address such a "fundamental" issue of the tool. I really appreciate it!

I haven't debugged the new code yet so I have a few clarification questions.

[timocov]

Is it a coincidence that iterating over renaming with i that is an index in declarations works because we find the first item? Can you clarify the relation between items (their positions) in renaming array and order of items in node.declarationList.declarations?

[Atrue]

Sure. Let’s take this statement as an example:

declare const a = 1, b = 2 (*)

this statement has 2 variable declarations:

a = 1 and b = 2.

Let’s assume then we need to rename b variable name to b$2, so according to TS api we need to need to create variable statement with 2 variable declarations (a = 1 and b$2 = 2)

The statement (*) goes to substituteNode and gets the renaming array for each statement inside declarations using getStatementRenaming.

dts-bundle-generator/src/bundle-generator.ts

Line 206 in 6c033c0

return statement.declarationList.declarations.map(declaration => {

So we get an array of [undefined, 'b$2'].

Then it goes inside custom createVariableStatement function and updates the name for each non-undefined value in the array:

declaration a = 1 will be the same as it has no renaming for index = 0

declaration b = 2 will be renamed to b$ = 2 with ts.factory.updateVariableDeclaration using a new name b$2 for index = 1.

[timocov]

is it (string | undefined)[] or string[]? what does it mean if an item is undefined?

[Atrue]

See #268 (comment)

[timocov]

is it condition true for every node with declare keyword or it should be a more specific check here?

[timocov]

looks like it will be applicable to statements like export declare interface Foo {} as well? if so, how it would work with declaration merging?

[timocov]

looks like in situations like this the renaming would be applicable to a variable declaration only which would lead to a incorrect output:

file1.ts:

export const MergedSymbol = '';
export interface MergedSymbol {
	test(): void
};

file2.ts:

export const MergedSymbol = '';
export interface MergedSymbol {
	test2(): void
};

index.ts:

export { MergedSymbol as MS1 } from './file1';
export { MergedSymbol as MS2 } from './file2';

Expected output:

declare const MergedSymbol = '';
interface MergedSymbol {
	test(): void
};

declare const MergedSymbol$2 = '';
interface MergedSymbol$2 {
	test2(): void
};

export {
	MergedSymbol as MS1,
	MergedSymbol$2 as MS2,
};

Actual output:

declare const MergedSymbol = '';
interface MergedSymbol {
	test(): void
};

declare const MergedSymbol$2 = '';
interface MergedSymbol { // note that this symbol isn't renamed but should be
	test2(): void
};

export {
	MergedSymbol as MS1,
	MergedSymbol$2 as MS2,
};

Even if the output compiles logically it is incorrect as it leads to wrong declaration merging.

[Atrue]

Honestly, it’s first time I see the interface statement with declare keyword export declare interface Test {}

You are right, this statement is also covered by modifiersMap[ts.SyntaxKind.DeclareKeyword] but getStatementRenaming has the additional check for ts.isVariableStatement(statement) so interface Test will never be renamed (it returns an empty array)

dts-bundle-generator/src/bundle-generator.ts

Line 205 in 6c033c0

if (ts.isVariableStatement(statement)) {

Note: A type/interface can not be renamed like this as it has much more problems: all types are reusable so they might be used in another type, function, class or something. It means it requires much more logic to check every related type in recreateRootLevelNodeWithModifiersImpl. So my goal was to fix only export default issue.

I heard about ts-morph library that allows to do such renaming in one call, so if you’re interested in it https://ts-morph.com/manipulation/

[timocov]

Hi @Atrue,

Sorry for a very long delay.

First of all, I want to thank you for your contribution and the fact that you jumped into a new project and were able to address such a complex problem at least deserves a recognition.

Secondly, your contribution kicked me off to address the names collision problem in general (not just for default names) and it seems that I have a solution that at least passed all tests that the tool currently have, including yours and several others that I've added during the implementation. It took a while to make it work...

As the next step, I'll be finishing the implementation (I need to write a couple more tests and run the tool against some well-known consumers of the tool to make sure that I'm not missing anything) and publishing a PR.

Unfortunately, it seems that none of your changes in the tool preserved (tests are fine tho) after my changes (I had to do big and a touch painful refactoring), but to pay respect to your contribution and to make sure you'll be in the list of contributors to the tool I'm going to merge your PR right before my changes anyway. Please keep it open until then.

After everything above is completed, I think it would be a preparation for the next major version (as amount of changes is enormous).

I hope that is alright with you and my long silence didn't cause too much problem for you.

[timocov]

If you're interested, I just created a draft PR #270 (mostly to keep track of packages I need to validate these changes against, but feel free to take a look/review/pull and check for your use case).

[timocov]

The fix has been released in v9.0.0.