-
Notifications
You must be signed in to change notification settings - Fork 455
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 ability to use npm and node executables installed by node-gradle-plugin #1499
Comments
To move npm usage from configuration-phase to execution phase, we would need to move the Current prepareNodeServer impl: spotless/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java Lines 72 to 88 in 9df1809
Moving the We would need to find a solution that does integrate nicely with the current state caching mechanisms. Any hints or pointers for that @nedtwigg ? |
The function of spotless/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java Line 46 in bba3ea1
So moving Comparing this to the rest of Gradle, if you want a jar file you put it into a "configuration", and those get resolved at configuration time, not task execution time. I think the If the goal is integration with the |
You are probably right there. This is the way it should be set up. We might as well try and raise an issue on the project.
I think the goal here (as I understood the previous issues raised e.g. #1381, #1185, #1486) is a smother integration between the two plugins. I think we might even ease the way for #1480 (when we delay the |
The slow operation is We could build such caching inside of Spotless, I'm happy to merge and release such a feature. As a maintainer, I'll merge and release anything that makes a project better for end-users. But when it comes to my time as a coder, I try to only spend time on systems that have the best design I can see. If I can see a better design, then I would rather spend my time persuading the upstream component or even forking it so that the design can be good. Spotless is actually such a case - it was originally an Eclipse formatter maintained by somebody else, I submitted a PR that refactored the plugin into As far as the npm stuff goes, I'm a maintainer not a coder, so I'm happy either way :) But imo the right thing is a better node plugin. If the existing one doesn't want to improve, Spotless can recommend integration with a fork. |
Released in |
When using the
node-gradle-plugin
to install the node/npm binaries, we have a timing issue:node-gradle-plugin
installs missing node/npm binaries in the execution phase.npm install
(for preparing formatting servers) in the configuration phase.Given that scenario, spotless will always fail in a clean setup, when a user tries to configure spotless to use the binaries installed by
node-gradle-plugin
(though it might succeed in a dirty state, when installation of the binaries has already happened in a previous gradle run).Snippet that demonstrates the problem:
Calling
gradlew spotlessTypesriptApply
will fail with something along the lines:This issue is to discuss the situation and possible solutions.
(Thanks @jnels124 for bringing this up in #1381 )
The text was updated successfully, but these errors were encountered: