-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support named module wildcard re-export when generating typescript types #8088
base: v2
Are you sure you want to change the base?
Conversation
Can someone shed some light on why checks on some of the platforms fail? |
You can ignore those. The tests are flakey, as long as one of the integration test jobs passes, you're fine. |
@@ -1,3 +1,8 @@ | |||
export {createMessage} from './message'; | |||
export * from './other'; | |||
export {test as toUpperCase, default} from './test'; | |||
export * as Module1 from './module1'; |
There was a problem hiding this comment.
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};
}
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
hey, any timeline for getting this out of the door? |
... when generating typescript types
afa29db
to
b1765a6
Compare
Is this really still pending? |
↪️ Pull Request
This is a proposal for resolving #5911.
It allows to specify the following in an index.ts file:
It does not support 1) the case:
and, it does not supported 2) nested exports.
If someone can update the test cases for the two unsupported cases so that I can see the expected result, then let me know.
💻 Examples
🚨 Test instructions
✔️ PR Todo