autoCreateTasksFromPackageJsonScripts not used/having effect

[nbrugger-tgm]

setting autoCreateTasksFromPackageJsonScripts to false should prevent auto creation as per documentation

but the code creates the tasks regardless

https://github.com/PeredurOmega/GradleNodePlugin/blob/74ac0d163e42034d3b5d8e056c55e144467893f9/src/main/kotlin/NodePlugin.kt#L99-L103

[nbrugger-tgm]

This is a thought one since its not that easy/possible to create clean conditional tasks, see gradle/gradle#23623

There are multiple ways with up and downsides:

• registering in afterEvaluate: the tasks cannot be modified with named as users would expect
• wrapping the registration in an if: doesn't work since it bypasses the laziness of extesion configuration
• making autoCreateTasksFromPackageJsonScripts a method rather than a field, that when the user invokes it creates the task
  • good: tasks are created eagerly -> user can configure them
  • good: user has controll if they exist
  • good: works with gradles lazy model
  • bad: breaking change for the plugin
  • bad: the auto creation is now opt in instead of opt out
  • bad: is not entirely clean! When using configuration cache this would not work since changes to the package.json would not be seen as a reason to invalidate the configuration -> adding a script to backage.json would not create the task in gradle (for example you wouldn't be able to see it in the tasks task list or ide) same goes for removal of tasks from package json.
    • That said this problem is one the plugin has already so it won't be a regression
• using onlyIf/enable: The tasks are still created with can be annoying to deal breaking. If you have a build task already that needs to be named as such it will fail since package json would create the task again causing duplication conflict
• creating a seperately applicable plugin such as npm-task-generator: Probably the cleanest way to handle it with current gradle capabilities but the split would bring obvious downside

[PeredurOmega]

Thanks for the input, I'll have a look at what would be the most appropriate way to do this.
For now my stand is the following:

• registering in afterEvaluate: wouldn't do for the reason you mention
• wrapping the registration in an if: would not work as you said
• making autoCreateTasksFromPackageJsonScripts a method rather than a field, that when the user invokes it creates the task: to me the opt in instead of opt out is a deal breaker on this one
• using onlyIf/enable: easiest, I'm willing to do this but as stated that might not be appropriate for most use cases I guess
• creating a seperately applicable plugin such as npm-task-generator: I might have a look at this but then again we might need to opt in instead of opt out.

I'll also try to think of other solutions.

Out of curiosity, why don't you want to auto generate those tasks ? Any particular use cases (as I never had to do that on my side and I have multiple projects using this at work) ?

[nbrugger-tgm]

Well foremost I discovered it because I already had a build task and the plugin therefore created a conflict.

The reason I do not "want" them is i do not need them. And it kind of is offputting to me to have many tasks that just not work correctly since they are not configured (missing inputs/outputs, missing ignoreExitValue = false, ...) so I have tasks that do not work (or work unexpected wrongly). While I might know this now, it won't be obvious to others.

The second reason is that the names are now reserved. I cannot create a "build" task with a different type. If i want a "build" task that builds a docker image i cannot (except doing it over npm which has its own problems). Its easy to say "just call it buildDocker then" but that has downsides as well. For example that now in ci one would need to do ./gradlew build :some:node:module:buildDocker when build is in the whole project seen as "build the docker image". This is an example and it is not a dealbreaker to me but i see it becoming annoying and my company not wanting it.

Besides that it is ok-ish to not have the option to turn it off but then a property named "disableThisBehaviour" should not exist and be documented as such.

BUT i have a proposal.
The plugin split can be done opt-out AND backwards compatible. The idea would be to extract a "lesser"/base version. A io.github.pereduromega.node.base-plugin. That provides the task types, the extension the execution-service. I could include the base plugin and customize myself away while existing users would be unaffected!