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
Generate files into different output units #237
base: main
Are you sure you want to change the base?
Generate files into different output units #237
Conversation
@danielkaneider thanks for your interest in typescript-generator and this PR but this is functionality I don't want to add to typescript-generator. Splitting output to multiple files increases complexity of already complex problem. Currently typescript-generator supports following module/namespace variants: http://www.habarta.cz/typescript-generator/doc/ModulesAndNamespaces.html. All these variants should be supported when splitting output so it would add another dimension to this problem (I can imagine splitting by package, by declaration and maybe some others). While I see you did very good job with this PR I still think it is not worth the increased complexity. Output splitting not only increases complexity of typescript-generator code but also increases complexity for typescript-generator users (because they would have more variants to explore, more configuration values). As a Java developer I know people are used to have each class in separate file but in TypeScript/JavaScript it is not often the case. In my opinion here splitting large output file into bunch of smaller files doesn't provide much benefits, there are still many declarations just organized differently. I think one output file has some advantages. It is easier to track in source control (if it is tracked), it is simpler to use (import), simpler to put into some documentation or just send in email. TypeScript compiler itself is good example of not splitting code into many source files. I know many people will disagree. For me it is also important to organize my code as I wish but in this case it is just generated code. I think in general it is good to have all possibilities hot to write code - bunch of smaller TypeScript files or one large etc. but again I think this is different case so it should not matter so much. To all readers: feel free to use 👍 or 👎 to provide feedback. |
@vojtechhabarta I know that the generator supports modules and namespaces. I see the different output units as a replacement for namespaces for example, not as an addition. Ambient modules might be different, but it shouldn't be hard to add support for them, or at least add some validation that the combination is not supported (yet). Let me explain the reason for this PR. I like the generated output very much, since it is very neat. But it is a little bit cumbersome to use, from a Java and TS perspective. We've been using namespaces in order to avoid name collisions. We used the generated type(s) like: The more elegant usage of types would just be I also thought about adding all the logic into one extension, but that would have gotten even worse. Please let me know if you have any idea how we could make that thing less complex |
I see two possibilities how to deal with large set of classes: Resolving name conflicts using
|
I'm aware of those options. Both are not really usable in an enterprise application: the first one needs the be adapted every time the generator fails. The second one is, as you pointed out, cumbersome to use due to bad IDE support. And then I'm wondering why namespaces (which are not IDE friendly) are supported within the generator, whilst file name imports are not. The latter one is quite common, too, and has also good support in IDEs. |
Nice PR. Having this functionality as a extension makes it much more accommodating to those who do not wish to adopt the added complexity. That being said, adding tests to support the added complexity of this extension would be beneficial. |
@jtoplak I also thought about an extension first. For that to work, the Emitter framework must be more flexible and interchangeable. I'm not sure if such a PR would be more accepted. |
Making Emitter part more extensible would be good. I don't know what exactly would be needed to allow this extension to work but maybe it could be done without too much changes just for this particular extension. |
238eae0
to
a537b17
Compare
Recently I cleaned the PR a little bit up, in order to rebased more easily for future versions. A few remarks to the current
Some suggestions
And I'm aware that some of the things might break backward-compatibility. |
a66f35b
to
8a069ab
Compare
The idea was to split the output into separate files, instead of putting everything into one output. First, in bigger projects a lot of files might get generated. Second, the
mapPackagesToNamespaces
wasn't really handy when it comes to using it actually in typescript.Comments to the implementation:
outputFileType
has been extended. A slightly cleaner (but breaking) solution would have been, to change the mapPackagesToNamespaces tomapPackages = {toNamespaces, toFiles}
for example