parshap/check-node-version

Odd versions cause the module to crash

GabrielLomba opened this issue · 5 comments

GabrielLomba commented

Hi.

I have faced an issue with this module recently where a computer had some really odd yarn version (yarn --version returned "0.32+git") and this line ended up throwing a TypeError: Invalid Version: 0.32+git.

Can we start checking for valid versions on commands output? I can submit a PR with this fix, if you want.

parshap commented

Hey @GabrielLomba, having some safety around this makes sense. I'd definitely appreciate and accept a PR for this.

c-vetter commented

That is indeed bad, I'm shocked that yarn would output a non-semver version. What would be the desired behaviour here?

We could:

  1. abort with a nicer message
  2. have the check fail with a message that the tool in question returned a non-semver version
  3. normalize the missing parts to .0 so that 0.32+git would be considered 0.32.0+git, and 0+git would be 0.0.0+git

Personally, I tend towards 2 in order to make these cases obvious. Behaviour 3, however, would probably be close to the intuitive expectation of most users.

We could combine 2 and 3 by introducing a flag like --loose to switch from 2 to 3.

@parshap What do you think?

parshap commented

Good point about intuitive expectations. That said, I agree 2 makes most sense. I don't think there's enough need for this to warrant the complexity of 3 or a --loose flag.

GabrielLomba commented

Agree with @parshap . Option 2 will make the error apparent right away and force the user in question to do something about it, not leading to some undefined state if we proceed with parsing non-semver versions.

I will probably send the PR this weekend.

c-vetter commented

@GabrielLomba already done 🙂 have a look at #38

@parshap please review and merge #38 after #37 has passed muster