spotless/prettier - .npmrc seems to be ignored

[dlangerenken]

My company is using a different registry for npm, specified by .npmrc:

registry = https://artifacthub.company.com/api/npm/npm-virtual/
sass_binary_site = https://artifacthub.company.com/node-bindings/4.10.0/
disturl = https://artifacthub.company.com/nodejs/

It appears that spotless is not picking those up. Is there any way I can add this as parameter to the prettier format? (essentially it needs to run npm install -registry=.... ...)

[nedtwigg]

What version of Spotless are you using? Our approach to npm changed in 4.3.1, so it would be helpful to know what you're seeing in 4.3.0 vs 4.3.1.

[nedtwigg]

Probably you'll need to make a change somewhere around here

spotless/lib/src/main/java/com/diffplug/spotless/npm/NpmProcess.java

Lines 36 to 38 in 5421f58

	void install() {
	npmAwait("install", "--no-audit", "--no-package-lock");
	}

Happy to take a PR for this. No-obligation ping to @simschla just in case you have a broader vision for how you'd like to handle this :)

[dlangerenken]

Thanks for the quick response @nedtwigg - I tried both 4.3.0 and 4.3.1 and it pretty much just hangs in

:spotlessInternalRegisterDependencies

A workaround I found was to have spotless dependOn a custom task that runs "path/to/npm" config set registry my-registry

[simschla]

No-obligation ping to @simschla just in case you have a broader vision for how you'd like to handle this

Personally, I've never used the npmrc. Based on the documentation, the most project-specific approach is to have a .npmrc file next to the package.json when running npm.

I think this would fit into the current api, using a similar approach to the npmExecutable setting:

prettier().npmExecutable('/usr/bin/npm').npmrc('/path/to/my/.npmrc').config(...)

(Another approach would be to try do autodiscover the .npmrc file by looking at the locations ${project.projectDir}/.npmrc, ${rootProject.projectDir}/.npmrc and $HOME/.npmrc - but even such an approach would need a possibility to override the location)

What do you think?

[dlangerenken]

I like the idea of being able to set .npmrc in a chained manner. Seems like the most flexible solution.
Best case of course would be to auto-detect the npmrc + have the .npmrc() method to override the auto-detection (or e.g. .npmrc(null) to disable it).

[dlangerenken]

Hi @simschla @nedtwigg - thinking a bit more about this problem, I'd also like to provide the node distribution myself. We're using https://github.com/node-gradle/gradle-node-plugin/issues in addition to spotless and would love to simply point to their node/npm distribution

This would ideally mean, that the prettier() chain contains two new parameters:

prettier().npmExecutable('/usr/bin/npm').nodeExecutable('/usr/bin/node').npmrc('/path/to/my/.npmrc').config(...)

I can definitely try to open a PR for this, although I'm not 100% sure how the node executable would be handled. I assume instead of calling npm install, it would be node npm install?

[simschla]

@dlangerenken From within spotless, we only ever call npm, which, as I understand it, has its own dependency to the node executable. So providing both executables seems redundant to me. However, how to exactly use the npm executable from node-gradle is beyond my knowledge. But I'm pretty sure the users or creators of node-gradle might be able to assist there.

[dlangerenken]

As far as I can tell from looking into the code, they do run the node executable followed by npm + additional commands. So essentially node npm start or similar.

See https://github.com/node-gradle/gradle-node-plugin/blob/master/src/main/kotlin/com/github/gradle/node/exec/NodeExecRunner.kt#L37 where it is passed into the execution-context (difficult to understand..)

What do you suggest we do? Do you think adding a way to configure the nodeExecutable could be part of this plugin or would you rather have this not handled by spotless? I'm happy to contribute through a PR if this is something desirable. It certainly is for my use case

[simschla]

What do you suggest we do? Do you think adding a way to configure the nodeExecutable could be part of this plugin or would you rather have this not handled by spotless? I'm happy to contribute through a PR if this is something desirable. It certainly is for my use case

Currently I don't have an opinion about this as I'm still trying to understand the usecase and/or benefits. Spotless itself does not trigger the node-executable directly, instead it calls the npm-executable. Are you suggesting that we change that and use the node-executable directly?

[dlangerenken]

Only as an optional setting. Even the gradle-node-plugin will default to the local node distribution, but it does have a download option.

#!/bin/bash
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
$DIR/node $DIR/npm "$@"

I actually got around the node-issue with this workaround. Instead of pointing to the npm executable, I'm simply pointing to the executable via the node executable. Currently seeing The file did not appear within PT1M but this seems unrelated.

[dlangerenken]

Okay, the The file did not appear within PT1M is related to this issue. While I can install the packages just fine using the bash workaround from above, I cannot run "npm start" since the package.json refers to the system-wide node:

https://github.com/diffplug/spotless/blob/main/lib/src/main/resources/com/diffplug/spotless/npm/prettier-package.json#L8

Is there any workaround such that I can override this value?

[simschla]

Ah, now I see your problem. Reading up on the topic it seems that calling npm directly is totally the way to go. npm does a lot in terms of trying to prepare the environment for having correct and as local as possible paths.

Based on Stackoverflow, you could problably use your previous workaround

"path/to/npm" config set registry my-registry

here too:

/path/to/npm config set scripts-prepend-node-path true

On spotless side, we could change these Lines

spotless/lib/src/main/java/com/diffplug/spotless/npm/NpmProcess.java

Lines 40 to 42 in 183c0b4

	Process start() {
	return npm("start");
	}

to

Process start() {
	return npm("start", "--scripts-prepend-node-path=true");
}

and that should make sure, that the node-binary relating to the npm binary in use should always be first on the path. no matter how the user has set things up in his environment.

[dlangerenken]

Hi @simschla, this might does the trick. Since it's configurable through .npmrc, I suggest we focus on adding the support of this file instead of hardcoding it into NpmProcess.java. I don't know what the implications are, but I assume it could potentially break for other users who don't want the path prepended.

[simschla]

Sorry I didn’t get around to work on this before my summer holidays. I’ll get to it in about two or three weeks. I hope you can live with the workarounds until then.

[dlangerenken]

Thanks @simschla - no rush. We are currently not using prettier with spotless and can wait a few more weeks until the .npmrc feature ends in a release!

Happy holidays.

[simschla]

Guys, I have a working prototype: main...simschla:feature/add-support-for-npmrc

I'm currently struggling writing an integration test for this .npmrc-Support. Especially I can't get the output of the node-servier via the GradleRunner to check if options in the .npmrc file have been picked up. @nedtwigg Any idea on how to achieve this?

[nedtwigg]

I'm okay merging it without an integration test. Testing system-level configuration is hard, sometimes there's no substitute for manual testing.

[simschla]

ok, so I'll update the docs and create the PR.

[simschla]

Fixed in #727.

[nedtwigg]

Published in plugin-gradle 5.8.1 and plugin-maven 2.6.0.