Find elm-format binary more reliably

[dillonkearns]

The code for finding the elm-format executable is unreliable and doesn't work in some environments. See the relevant code here:

elm-graphql/generator/src/formatted-write.ts

Lines 4 to 5 in 38860e6

	const globalInstallPath = `${__dirname}/../node_modules/.bin/elm-format`;
	const localInstallPath = `${__dirname}/../../.bin/elm-format`;

elm-graphql/generator/src/formatted-write.ts

Lines 55 to 63 in 38860e6

	const findElmFormatPath = (): string | null => {
	if (fs.existsSync(globalInstallPath)) {
	return globalInstallPath;
	} else if (fs.existsSync(localInstallPath)) {
	return localInstallPath;
	} else {
	return null;
	}
	};

It would be nice to find a way to reliably find the executable so that the elm-format step can be run for everybody.

[btorellALTA]

I'm having this issue on MacOS 10.13 High Sierra with a global install of elm-format@0.8.1. I'm running elm-graphql as an NPM script, like so:

...
scripts: {
    "api": "elm-graphql <my GraphQL server> --base Api --output elm/src"
}
...

[dillonkearns]

Thank you for providing those details @btorellALTA!

[BrianHicks]

random kibitzer here! What's wrong with using the system lookup mechanism, e.g. just calling elm-format directly? That would find it on PATH, wherever that actually shows up. If elm-format is not installed, you'd catch an ENOENT-style exception.

edit: As a bonus, it's safe to assume that PATH is well-ordered, so PATH=/path/to/project/node_modules/.bin:/usr/local/bin:/usr/bin:/bin would look in each of those locations in turn. I always add node_modules/.bin to my PATH using direnv for this reason—things mostly Just Work™

[dillonkearns]

@BrianHicks do you think that npx elm-format would work well? I think it's reasonable to assume that a user has a relatively up-to-date npm version (and therefore has npx available).

Ideally we want to ensure that they are using an elm-format version with certain constraints, rather than whatever they have installed globally. For example, let's say there's a new release of Elm, and they have a globally installed elm-format that only supports 0.19 or below. If they're getting errors suddenly, it would be reasonable for them to open up a bug report here. So it would be nice to be able to ensure a constraint on the version, and make sure the local version is used ideally!

[BrianHicks]

npx elm-format will not work if someone installed elm-format without npm, will it? For example, both Homebrew and Nix have elm-format available.

In that case, it seems better to use the environment to do this in the standard way rather than try and do something fancy-but-breaking in the CLI.

Fortunately, elm-format --elm-version {0.18,0.19} exists. If you call it with an unsupported version (say 0.20) it fails early and with a clear message, attached:

$ elm-format --elm-version 0.20 src/Main.elm
option --elm-version: Invalid Elm version "0.20".  Supported versions are 0.18, 0.19

elm-format 0.8.1

Usage: elm-format [INPUT] [--output FILE] [--yes] [--validate] [--stdin]
                  [--elm-version VERSION] [--upgrade]
{ … snip … }

It seems like that would address the concerns you had above? But maybe not?

(also I'm kibitzinggggg! Do what you think best, as always. ❤️)

[dillonkearns]

@BrianHicks your comments are much appreciated!

Now that you explain that, you're totally right that npx assumes that they have it installed locally... which is not very polite to have a hidden required dependency like that!

elm-format is listed as a dependency of the @dillonkearns/elm-graphql npm package... and what I really want to do, in an ideal world, is to point at that binary from the script! That would simplify things a lot. I'm having trouble googling it, I'm not sure if that's even possible or not... 🤔

[BrianHicks]

well, iff the CLI must be installed via npm, AND it's also a hard dependency (i.e. not a peer dependency) AND you can assume a sufficiently modern npm installation, then npx may make sense!

[JonRowe]

I'm also using a script in package.json:

  "scripts": {
    "format": "elm-format src/",
    "update_graphql": "elm-graphql 'http://0.0.0.0:4001/graphql' --header \"Authorization: Bearer $TOKEN\" --base App"
  },

It's worth pointing out that my elm and everything is all in node_modules so I have no global tools, and I don't have node_modules/bin on my path so I've been using npm run or yarn run. Only learnt about npx today!

[dillonkearns]

Hello all, thank you everyone for the excellent feedback! You all rock! 💯

It sounds like the npx route with what NPM calls a peer dependency will solve the problem well. With the NPM peer dependency, npm install will tell you you need to add elm-format as a dependency before proceeding if it's not already there. We've got a working branch with this implemented:

https://github.com/dillonkearns/elm-graphql/pull/87/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R43

I'll probably publish it today, unless I hear any strong disagreements. Thanks all!

[dillonkearns]

#87 has been merged! I'm feeling good about this solution. Please send me feedback if you run into any issues here, I think this should resolve the problem though 👍 Thanks all!

[dillonkearns]

@dillonkearns/elm-graphql version 1.0.8 is live on NPM!