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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support named module wildcard re-export when generating typescript types #8088

Open
wants to merge 2 commits into
base: v2
Choose a base branch
from
Open

Support named module wildcard re-export when generating typescript types #8088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be2bb59
Support named module wildcard re-export
rmja May 12, 2022
afa29db
Merge branch 'v2' into wildcard-module-export
rmja May 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/core/integration-tests/test/integration/ts-types/exporting/expected.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ export var a: number, b: number;
export function toUpperCase(foo: string): string;
declare const _default: "test";
export default _default;
export declare module Module1 {
export const hello = "world";
}

//# sourceMappingURL=types.d.ts.map
5 changes: 5 additions & 0 deletions packages/core/integration-tests/test/integration/ts-types/exporting/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
export {createMessage} from './message';
export * from './other';
export {test as toUpperCase, default} from './test';
export * as Module1 from './module1';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding export {hello} from './module1' here results in the types being duplicated, both outside and inside the module block. For strings it's not too bad, but for more complex types might be weird. Maybe we should still hoist the declarations out of the module as before, but create a module like this instead to export them?

export const hello = "world";
export declare module Module1 {
  export {hello};
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good idea. That could probably also include support for the not supported 1) above.
The problem is then that we very likely gets naming conflicts between the hoisted definitions and the exports. Should we generate some random part in the name to avoid these conflicts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the hoisting we already do should handle renaming in the top level scope. So you might need something like export {hello1 as hello} within the nested module in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devongovett could I ask you to roughly commit to this PR the (failing) tests that describes the intended behavior, then I will try and do the implementation.


// TODO: Make this work
// import * as Module2 from './module2';
// export { Module2 };
4 changes: 4 additions & 0 deletions packages/core/integration-tests/test/integration/ts-types/exporting/module1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const hello = "world";

// TODO: Make this work
// export * as Nested from "./nested-module";
4 changes: 4 additions & 0 deletions packages/core/integration-tests/test/integration/ts-types/exporting/module2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const hello = 2;

// import * as Nested from "./nested-module";
// export { Nested };
1 change: 1 addition & 0 deletions packages/core/integration-tests/test/integration/ts-types/exporting/nested-module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const nested = true;
2 changes: 1 addition & 1 deletion packages/core/integration-tests/test/ts-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe('typescript types', function () {
assertBundles(b, [
{
type: 'js',
assets: ['index.ts', 'message.ts', 'other.ts', 'test.ts'],
assets: ['index.ts', 'message.ts', 'module1.ts', 'other.ts', 'test.ts'],
},
{
type: 'ts',
Expand Down
9 changes: 9 additions & 0 deletions packages/transformers/typescript-types/src/TSModuleGraph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,15 @@ export class TSModuleGraph {
return null;
}

if (e.imported === '*') {
// Named, wildcard re-export, e.g. export * as Something from "./something-else"
return {
module: m,
imported: '*',
name: exportName,
};
}

let exp = this.resolveExport(m, e.imported);
if (!exp) {
return null;
Expand Down
24 changes: 12 additions & 12 deletions packages/transformers/typescript-types/src/collect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,19 @@ export function collect(

if (ts.isExportDeclaration(node)) {
if (node.exportClause) {
for (let element of node.exportClause.elements) {
if (node.moduleSpecifier) {
currentModule.addExport(
element.name.text,
(element.propertyName ?? element.name).text,
node.moduleSpecifier.text,
);
} else {
currentModule.addExport(
element.name.text,
(element.propertyName ?? element.name).text,
);
if (ts.isNamedExports(node.exportClause)) {
for (let element of node.exportClause.elements) {
const imported = (element.propertyName ?? element.name).text;
const specifier = node.moduleSpecifier?.text;
currentModule.addExport(element.name.text, imported, specifier);
}
} else if (ts.isNamespaceExport(node.exportClause)) {
// Namespace export, e.g. export * as Something from "./something-else".
currentModule.addExport(
node.exportClause.name.text,
'*',
node.moduleSpecifier.text,
);
}
} else {
currentModule.addWildcardExport(node.moduleSpecifier.text);
Expand Down
10 changes: 9 additions & 1 deletion packages/transformers/typescript-types/src/shake.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ export function shake(

moduleStack.push(_currentModule);
let isFirstModule = !_currentModule;
_currentModule = moduleGraph.getModule(node.name.text);
const nodeModule = moduleGraph.getModule(node.name.text);
_currentModule = nodeModule;
let statements = ts.visitEachChild(node, visit, context).body.statements;
_currentModule = moduleStack.pop();

Expand All @@ -59,6 +60,13 @@ export function shake(
addedGeneratedImports = true;
}

const exportedName = nodeModule.names.get('*');
if (exportedName) {
node = ts.getMutableClone(node);
node.name = ts.createIdentifier(exportedName);
node.modifiers.unshift(ts.createModifier(ts.SyntaxKind.ExportKeyword));
statements.push(node);
}
return statements;
}

Expand Down