-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Star re-exports (import * as NS
/ export * as NS
) aren't wrapped with a NS
name
#134
Comments
Probably it was broken in 5.4.0 in #132. As workaround you can try to use export * as R from "ramda"
export * as cheerio from "cheerio"; |
@timocov, i've uploaded a repository with some tests i've made, i also added an example with the export star, which didn't worked :/ |
@schleumer Thanks for the repo with tests! Is it intended that you'd like to inline these libraries into a bundle, right? |
Just wonder what output here could be... Wrapped |
import * as NS
/ export * as NS
) aren't wrapped with a NS
name
@timocov I read through this issue and I think that this issue (#134) and the issues #139 and #146 describe different problems. From what I understand, this issue is about that namespaced imports to external modules. However, #139 and #146 are about namespaced imports to local files. This bundler has to generate different code in those situations. While external modules should be referenced, local files have to inlined. I do not know whether #134, #139, and #146 are all caused by the same underlying issue but they do require different solutions. To answer your questions as to what the correct output should be: This issueThe input code of #134: import * as R from "ramda";
import * as cheerio from "cheerio";
export { R, cheerio } should have the output: import * as R from "ramda";
import * as cheerio from "cheerio";
export { R, cheerio } This is what I would say is correct. This is a valid The other issuesThe input code of #139 (slightly modified to make it more clear): // index.ts
export type A = number;
export * as Bar from "./bar";
// ./bar.ts
export type B = number; should have the output: export declare type A = number;
export declare namespace Bar {
export declare type B = number;
}; The input code of #146 (credit to @bradzacher): // file1
export interface Foo {}
export interface Bar {}
// file2
export * as Test from './file1'; should have the output (slightly modified by me): export declare namespace Test {
export declare interface Foo {}
export declare interface Bar {}
} |
Added a failing test for this #154, trying to fix the actual issue too but having some problems understanding TS AST and where to put the fix 😓 |
@alexandernanberg If you have trouble understanding TS AST, then AST explorer might be helpful. Unfortunately, I can't help you with the fix. |
@RunDevelopment did you see that https://github.com/schleumer/dts-bundle-generator-tests/blob/e4a16a709215765dad6ba29c409336d428e24e01/broken-latest-ramda-and-cheerio/bundle.js#L17 these libraries are expected to be inlined, so we might treat them like a "internal" ones.
This is pretty easy case, but the problem is to handle something like this: // index.ts
import { Foo } from './foo';
export type A = number | Foo;
export * as Bar from "./bar";
// bar.ts
export type B = number;
// foo.ts
import { B } from './bar';
export type Foo = B | string; As you can see here we have 2 types of accessing to types from |
I didn't. My bad. In this case, all three issues are equivalent.
@timocov I see that your example is a little more complex but it is still solvable. type Foo = Bar.B | string;
export type A = number | Foo;
export declare namespace Bar {
type B = number;
}; That being said, your example most certainly isn't easy to solve in general. The main problem is that the dependencies form a DAG (let's hope it's acyclic) which is much harder to handle than a tree. I don't know whether this will affect how hard/easy the problem is to solve, but maybe it's enough to only support dependency trees for namespaces for now?
Does renaming include adding namespace prefixes? From what I see, the bundler only needs to add namespace prefixes.
"it" - the renaming or the namespace creation? |
Probably in this case we can add another limitation to the list in readme (if you use // index.ts
import { Foo } from './foo';
export type A = number | Foo;
export type B = string;
export * as Bar from "./bar";
// bar.ts
export type B = number;
// foo.ts
import { B } from './bar';
export type Foo = B | string; In this case, if we just "wrap" a whole module into export type Foo = B | string;
export type A = number | Foo;
export type B = string;
export namespace Bar {
export type B = number;
} which will compile, but has completely wrong meaning because Another thing we have to worry about is to do "tree shaking" inside the namespace, so we have remove all unused types from that "artificial" namespaces (currently the tool doesn't have such feature for "natural" namespaces though, but since we're talking about generated ones I think we need to worry about that).
Yes. If we're talking about handling all cases, we need to think about "true-renaming" with following the dependencies tree.
Oops, sorry for confusing. Renaming for sure. |
Can't we fail instead of producing wrong code? People (e.g. I) rely on this being correct.
Frankly, I would love to do a deep dive into this. This seems like an interesting problem but I don't have the time right now. If this is still a problem in a few weeks from now, I'll do a PR. |
If we can detect this then yes, I'd prefer to handle this and report an error for that for sure. |
Hello @timocov does this conditionals help solve:
|
@wolfram77 can you provide examples for each please? I'm not sure that I understood your point correctly, I'm sorry. |
@timocov Please check the examples taken from above. Example 1
Original input: // bar.ts
export type B = number;
// index.ts
export type A = number;
export * as Bar from "./bar"; This can be rewritten by wrapping declare namespace ts_bar {
export declare type B = number;
}
export declare type A = number;
export ts_bar as Bar; Since namespace export declare type A = number;
export declare namespace Bar {
export declare type B = number;
} Example 2
Original input: // file1.ts
export interface Foo {}
export interface Bar {}
// file2.ts
export * as Test from './file1'; Can be rewritten as: declare namespace ts_file1 {
export declare interface Foo {}
export declare interface Bar {}
}
export ts_file1 as Test; Which can then be simplified as: export declare namespace Test {
export declare interface Foo {}
export declare interface Bar {}
} Since there are no other usages of namespace Example 3
Original input: // bar.ts
export type B = number;
// foo.ts
import { B } from './bar';
export type Foo = B | string;
// index.ts
import { Foo } from './foo';
export type A = number | Foo;
export * as Bar from "./bar"; Can be rewritten as (wrapping into namespace): declare namespace ts_bar {
export declare type B = number;
}
declare namespace ts_foo {
export declare type Foo = ts_bar.B | string;
}
export declare type A = number | ts_foo.Foo;
export ts_bar as Bar; Which can then be simplified as: export declare namespace Bar {
export declare type B = number;
}
declare namespace ts_foo {
export declare type Foo = Bar.B | string;
}
export declare type A = number | ts_foo.Foo; Now only export declare namespace Bar {
export declare type B = number;
}
declare type Foo = Bar.B | string;
export declare type A = number | Foo; Example 4
Original input: // bar.ts
export type B = number;
// foo.ts
import { B } from './bar';
export type Foo = B | string;
// index.ts
import { Foo } from './foo';
export type A = number | Foo;
export type B = string;
export * as Bar from "./bar"; Can be rewritten as: declare namespace ts_bar {
export declare type B = number;
}
declare namespace ts_foo {
export declare type Foo = ts_bar.B | string;
}
export declare type A = number | ts_foo.Foo;
export declare type B = string;
export ts_bar as Bar; Which can then be simplified as: export declare namespace Bar {
export declare type B = number;
}
declare namespace ts_foo {
export declare type Foo = Bar.B | string;
}
export declare type A = number | ts_foo.Foo;
export declare type B = string; Which can then be re-simplified as ( export declare namespace Bar {
export declare type B = number;
}
declare type Foo = Bar.B | string;
export declare type A = number | Foo;
export declare type B = string; Example 5
import * as R from "ramda";
import * as cheerio from "cheerio";
export { R, cheerio } Can be simplified to: export * as R from "ramda";
export * as cheerio from "cheerio"; I would like to fix the conditionals:
At each step, symbols need to be renamed appropriately. |
Idea from @wolfram77's seems like feasible. For example TS AST representation of following is:
We could get identifiers from typescript AST, asterixes (if needed at all since we know it's namespace export) etc. I have same problem in my project. A named star (re)export is not included. As it's |
Is there someone on this actually? |
Hi everybody, I'm really sorry for not replying in this thread for a long time. I do really think that this and name collision problems (#184, #116 and #130) are the most complex problems in this tool (especially renaming one as it requires deep diving into typescript's AST manipulation which I don't have much experience with yet). At the same time, I do understand that these 2 problems are extremely important (and always have been since very first published version) and someone would prefer to use different tools (see #68) instead that might not have these problems. If I'm not mistaken, to transform AST in the form above it is not that simple as just "replace a node with a new node with different name", it requires to traverse every single node in the project and check if that renamed (or converted) node is referenced and then replace it accordingly. From my perspective, this is huge amount of work, effort and time which I am not always have, unfortunately. But I could be wrong and would be happy to be proven wrong if someone has understanding and vision how to implement it and can spend their time to do it. Another ideaMeanwhile, I think I might have an idea how to solve the problem without implementing renaming mechanism, but it would have another limitation. The idea is the following:
Limitations/open questions
ExampleI'll take one of the examples above, don't think there is a big difference between them for this solution, but feel free to post your ideas/examples in the replies. // bar.ts
export class Bar {}
// foo.ts
import { Bar } from './bar';
export type Foo = Bar | string;
// index.ts
import { Foo } from './foo';
export type A = number | Foo;
export type B = string;
export * as Bar1 from "./bar";
export * as Bar2 from "./bar";
export * as Foo from "./foo"; as the result it would be something like this: declare class Bar {}
declare type Foo = Bar | string;
export declare type A = number | Foo;
export declare type B = string;
export declare namespace Bar1 {
export { Bar };
}
export declare namespace Bar2 {
export { Bar };
}
export declare namespace Foo {
export { Foo };
} What do you all think about it? For me, it feels quite feasible in the matter of implementation and effort and it kinda solves the problem (partially tho). But I'm afraid that limitations might not satisfy these who really need this feature in full power, so I'll be waiting for feedback. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Hey all. I don't know if it will be of any help, but I wanted to drop a quick note on how I'd handle this if I were building from scratch. Quick caveats:
So there is my disclaimer. With that said, in case it helps... Walking Phase
something like... interface TypeData {
srcName: string
dstName: string
declaration: DeclarationNode
symbol: Symbol
sourceFile: SourceFile
children?: TypeData[]
isRootExported: boolean
} Notes:
Writing Phase
Notes:
Further NotesFWIW - I think trying to merge type declarations into unions, etc. would not be expected behaviour and would lead to issues. If I had two files with a type having the same name, I'd definitely prefer they not be merged. Instead, getting an error or a rename with warning is good. Having the option to handle via middleware would be great. I also think that having more logical separation between the walking and writing phases may help simplify things, but again, I've not fully familiarized myself with your code. In any event, thank you for the useful library! Hopefully this will come in handy, but if not, I look forward to seeing your eventual solution.
|
Bug report
I've used
dts-bundle-generator
to generate bundle of.d.ts
for use on Monaco Editor and some wierd Typescript compiler that runs on GraalJS that i've made. I have 2 globals which are actually aliases to npm modules, which are Ramda and Cheerio, they worked great on some old.d.ts
files, but when i've updated those libraries to its recent version, the code generation didn't work.Input code
Expected output
Something around this: https://gist.github.com/schleumer/3c0c7d468e050b2163760e7e4392d5cd, of course, those libs changed a lot and may be slightly different
Actual output
Additional context
I've made a custom script, something like:
Also, with internal files,
dts-bundle-generator
works great, my only issue is with importing Ramda and Cheerio, don't know if there's something new that i've lost.Thanks for the awesome library :)
The text was updated successfully, but these errors were encountered: