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.

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

Add output directory of all compile* tasks to typescript-gen classpath #205

Merged
merged 1 commit into from
Dec 31, 2017

Conversation

jechlin
Copy link

@jechlin jechlin commented Dec 29, 2017

Add samples for groovy, kotlin and scala to sample gradle file. In
addition, use the gradle wrapper to allow easily switching the gradle
version.

Note: An easy way for anyone who has the problem this fixes to test, is to use https://jitpack.io/.

Add to the end of your buildScript.repositories:

maven { url 'https://jitpack.io' }

And replace the plugin dependency with:

classpath 'com.github.jechlin:typescript-generator:6e4db65a4e3012e9d976ec7241e8cbe8e9e5b173'

Add samples for groovy, kotlin and scala to sample gradle file. In
addition, use the gradle wrapper to allow easily switching the gradle
version.
@jechlin
Copy link
Author

jechlin commented Dec 29, 2017

In cz.habarta.typescript.generator.gradle.TypeScriptGeneratorPlugin I replaced "classes" task with "build" task, to avoid problems with circular dependencies. However typically the ts generation should happen before the js-compile stage, eg webpack or whatever. But we don't know the task where the ts files will be used.

It may be better just to have the user specify the dependencies themselves in their build file, eg

tasks.generateTypeScript.dependsOn(["compileGroovy"])

and

tasks.webpack.dependsOn(["generateTypeScript"])

etc. That's what I do. But that would be a breaking change I guess so over to you.

Not totally confident in these changes, most of them are just samples to check the plugin works properly. You may want to take only changes to cz.habarta.typescript.generator.gradle.GenerateTask.

Maybe it could be possible to somehow recognize all "compile" tasks like compileJava, compileGroovy, etc. and use them or maybe we could use all tasks but I don't know if this could add unwanted files.

I don't think that's an issue. If you want 4 languages in your project you should be able to write interface definitions in any of the languages, and it's only one directory per language we are adding.

If we didn't have any solution which would work without configuration we could add configuration parameter which would specify list of "compile" tasks (something like classpathFromTasks) which would by default have value with list of common languages like java, groovy, kotlin, scala.

I think the easiest thing would be to allow to specify the classes output directory in the config, rather than the tasks. Eg classesDir = "$buildDir/classes/kotlin/main". Easy change but I didn't do it...

@jechlin
Copy link
Author

jechlin commented Dec 29, 2017

One final thing - the scala class doesn't produce any interface members, but I'm sure that's because I don't know how to write the scala class rather than any other problem.

@vojtechhabarta
Copy link
Owner

Thanks @jechlin for this PR.

It seems that this also relates to #161, pinging @r89m, @jtoplak, if you want to comment.

I would merge this PR without changes now and for typescript-generator version 2.0 (which I plan soon for #186) I would

  • leave generateTsTask.dependsOn("compile*") in code because typescript-generator really depends on compile tasks results
  • removed classesTask.dependsOn(generateTsTask) so it is more flexible

What do you think?

@jechlin
Copy link
Author

jechlin commented Dec 29, 2017

removed classesTask.dependsOn(generateTsTask) so it is more flexible

The problem then is the task won't run unless you explicitly specify the task in the gradle command line, eg:

gradle generateTypeScript build

because without that, there will be nothing that depends on generateTypeScript.

@r89m seems to be saying the same thing as me, which is just to avoid adding the dependencies and letting the users specify them in the build file via dependsOn etc. But I guess that is a breaking change. Most other plugins don't insert their tasks into the lifecycle.

@vojtechhabarta
Copy link
Owner

@jechlin yes, this would be breaking change (so I would wait with it till 2.0).

I understand that generateTypeScript task would not be invoked unless specified explicitly on command line. But people could also add for example tasks.webpack.dependsOn(["generateTypeScript"]) to their build files as you mentioned above. Or did I misunderstand you?

@jechlin
Copy link
Author

jechlin commented Dec 29, 2017

No, that is what I mean. But if they have to do that isn't that the same breaking change as making them specify tasks.generateTypeScript.dependsOn(["compileJava"]) ?

@vojtechhabarta
Copy link
Owner

Yes, the same kind of breaking change, but I think that generateTypeScript task should always depend on compile* tasks because it needs to use their outputs. Or do you think it is better to NOT have this dependency specified in TypeScriptGeneratorPlugin.java?

@jechlin
Copy link
Author

jechlin commented Dec 29, 2017

I'm not sure... I was trying to think of any task that would generate class files that wasn't a compile* task, and I couldn't. Therefore it seems to make sense to have the generateTsTask dependent on the compile* tasks, otherwise the user will always have to add that. So I would keep that and remove the classes.dependsOn(generateTs).

@vojtechhabarta vojtechhabarta merged commit d25f068 into vojtechhabarta:master Dec 31, 2017
@vojtechhabarta
Copy link
Owner

Hi Jamie,
I removed Gradle Wrapper as part of cleaning thinks before v2 release.
In Gradle documentation they state that "The Wrapper is something you should check into version control." but I don't like having copy&pasted files in the repo. I think having extra files is confusing for Gradle beginners while advanced users can easily add this wrapper if they want.

@vojtechhabarta
Copy link
Owner

Hi again,
just FYI: I also removed Gradle jar files that were copied in typescript-generator-gradle-plugin/gradle-lib (at the expense of adding https://repo.gradle.org/gradle/libs-releases-local/ Maven repository) - 9a700d3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants