node-gradle / gradle-node-plugin

Gradle plugin for integrating NodeJS in your build. :rocket:
Apache License 2.0
599 stars 117 forks source link

Can we consider dynamically registering gradle tasks based on npm scripts #295

Closed alexanderankin closed 8 months ago

alexanderankin commented 8 months ago

when the plugin is registered, it can check package.json and register any scripts as tasks. thus - instead of specifying the script in both package.json as well as build.gradle, they can just use package.json and magically refer to this task elsewhere in the project.

this seems like it would be very useful, curious what everyone else thinks.

deepy commented 8 months ago

I like this conceptually, but I suspect there might not be enough information in there to create good tasks The lifecycle ones seem like a good candidate, but as they're part of the lifecycle they're already automatically and hooking them into Gradle risks running them incorrectly or multiple times (e.g. once from gradle and then once from npm)

And for the other scripts, these usually have some form of inputs and/or outputs that aren't represented (in a parseable format) in package.json, and if you want quick and correct builds you really want to get your inputs and outputs right.

With that said, maybe a warning about the missing configuration is sufficient, or maybe having a task that templates the thing for you so you can copy-paste the output into your build and iterate from there.

Do you have some example scripts or even a package.json I can have a look at?

alexanderankin commented 8 months ago

Would it be acceptable to just define the tasks without hooking them up? So they appear as tasks.npmRunBuild or tasks.npmRunLint or something like that

Of course you can used named as well for lazy configuration loading.

The inputs and outputs is tricky, still thinking about this. But my inclination is that I forget to do this all the time anyways, and have to come back for a second pass. This approach above would at least eliminate the first pass, which is an improvement

On Tue, Nov 28, 2023, 3:39 AM Alex Nordlund @.***> wrote:

I like this conceptually, but I suspect there might not be enough information in there to create good tasks The lifecycle ones seem like a good candidate, but as they're part of the lifecycle they're already automatically and hooking them into Gradle risks running them incorrectly or multiple times (e.g. once from gradle and then once from npm)

And for the other scripts, these usually have some form of inputs and/or outputs that aren't represented (in a parseable format) in package.json, and if you want quick and correct builds you really want to get your inputs and outputs right.

With that said, maybe a warning about the missing configuration is sufficient, or maybe having a task that templates the thing for you so you can copy-paste the output into your build and iterate from there.

Do you have some example scripts or even a package.json I can have a look at?

— Reply to this email directly, view it on GitHub https://github.com/node-gradle/gradle-node-plugin/issues/295#issuecomment-1829341993, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACECGJCDUATEVJ4CVOWMMG3YGWPKNAVCNFSM6AAAAAA7ZYEFB2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRZGM2DCOJZGM . You are receiving this because you authored the thread.Message ID: @.***>

deepy commented 8 months ago

There's currently a Rule that allows you to run arbitrary npm commands, and it has a little bit of extra logic for the npm run foo case, so the following allows you to use ./gradlew npm_run_yourscript to run the yourscript from package.json

        project.tasks.addRule("Pattern: \"npm_<command>\": Executes an NPM command.") {
            val taskName = this
            if (taskName.startsWith("npm_") && enableTaskRules.get()) {
                project.tasks.create<NpmTask>(taskName) {
                    val tokens = taskName.split("_").drop(1) // all except first
                    npmCommand.set(tokens)
                    if (tokens.first().equals("run", ignoreCase = true)) {
                        dependsOn(NpmInstallTask.NAME)
                    }
                }
            }
        }

If you want to use it, I suggest you turn off the enableTaskRules and re-create this rule yourself, only allowing npm_run_ instead of any npm_. This rule is the biggest source of misconfiguration in this plugin and while I see why you'd want it, it really is more problem than its worth.

deepy commented 8 months ago

Closing it as the functionality already exists :-)