Skip to content
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 package-json subcommand #50

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Add package-json subcommand #50

wants to merge 16 commits into from

Conversation

jasonkarns
Copy link
Member

@jasonkarns jasonkarns commented Jan 21, 2019

Significant refactor to support the traditional subcommand/bin/libexec
structure of nodenv plugins.

  • Removes plugin_root from bin, so we aren't polluting user's PATH with more stuff.
  • adds nodenv-package-json which provides nodenv package-json subcommand. This subcommand presently is a "reader" for package.json just as nodenv local prints the version specified from local .node-version file. Intend to have this command also be a "writer" in the future.
  • Splits the finding and reading of package.json into two separate sub utilities (nodenv-package-json-file and nodenv-package-json-read) which are corollaries to nodenv-version-file and nodenv-version-file-read. The benefit is that the origin hook can now print the path to the actual package.json that is affecting the node version, along with the version_spec from said file. It invokes these utilities without needing to source anything.

Also addresses some latent bugs:

todo:

  • resolve all scenarios of PWD vs NODENV_DIR
  • ensure error is printed when no matching version found
  • update shellcheck to leverage extension-based linting

`nodenv-package` is the corrolary to nodenv-shell, nodenv-local, and
nodenv-global. Right now, it is just a subcommand to show the version
selected from package.json. In the future, it should also be used to
_set_ the version in package.json.
Separately; old code had a bug where the first conditional branch would
never be executed (the `get_version_respecting_precedence` function
never returned non-zero). This is actually a good thing, because we
wouldn't _want_ to print anything to STDERR in this hook, anyway.

Also, this hook script is sourced by nodenv so it shouldn't have a
shebang, nor be executable.

Lastly, we rely on nodenv ensuring each plugin's `bin/` dir is added to
PATH so we can just invoke `nodenv-package` directly.
Both the 3rd party deps are symlinked to bin, but we don't actually want
to lint them. Meanwhile, the only files that exist in bin that _should_
be linted, are symlinks to libexec. So we can just lint them there.
npm won't include any symlinks in the tarball, so we need to explicitly
include the main bin. Pointing to libexec allows npm to make the symlink
from node_modules/.bin/ -> libexec/

The other bins aren't necessary because they are provided by npm
automaticaly (and included in path that's to npm). (Assuming this is
installed globally, which would ensure the bins are in PATH.)
These vars are set by nodenv itself, so they'll likely only be set if
the tests were run via nodenv (as is the case with `npm t`).

Though other maintainers may have some of these set in the shell
profile, so we need to unset them regardless.
@jasonkarns jasonkarns added this to To Do in nodenv Aug 10, 2019
@jasonkarns jasonkarns moved this from To Do to In progress in nodenv Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
nodenv
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

1 participant