Skip to content

Conversation

@lukaselmer
Copy link
Contributor

@lukaselmer lukaselmer commented Jan 23, 2019

  • Only reshims when npm packets are installed which have a binary
  • I didn't find any documentation about the env variable npm_package_bin_*, but it seems that it is always npm_package_bin_<bin_name>, e.g. npm_package_bin_prettier for the package prettier
  • As pointed out by @augustobmoura, the documentation is here

@minusfive
Copy link

Wondering whether there are plans to merge this, or is there another solution to npm i -g in the works? Prepending the ENV var from #117 every time feels pretty hacky.

bin/postinstall Outdated
if [ "$ASDF_SKIP_RESHIM" == "1" ]; then
echo "Run 'asdf reshim nodejs ${ASDF_INSTALL_VERSION:-$npm_config_node_version}' after installing the packets to reshim"
else
elif [ `/usr/bin/env | grep "npm_package_bin"` ]; then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to replace this line with elif /usr/bin/env | grep -q "npm_package_bin_"; then for it to work.

@MrQubo
Copy link

MrQubo commented Apr 3, 2020

@minusfive You can add this to .bashrc, or equivalent if using shell other than bash.

@Stratus3D
Copy link
Member

I am not very familiar with this functionality, so I can't really review this PR for correctness. If any other contributors to asdf-nodejs have any thoughts on this please let me know.

@homburg
Copy link

homburg commented May 8, 2020

This seems to work correctly.

If @surajbarkale suggestion is included and merge conflicts are resolved, I'd vote for merging this.

@augustobmoura
Copy link
Member

augustobmoura commented Jan 22, 2021

The documentation is here, it only adds npm_package_bin_{executable} because the packages have a bin section in their package.json (every entry in the bin object will get a variable), which in turns means that it will add a global command that needs reshimming.

Would be better if the reshim only occurred after the last install, I will see if something can be done about that. The reshimming only on binaries is a advance anyways, @lukaselmer can you solve the issues with the PR? Or can we open another one?

@augustobmoura
Copy link
Member

#197 does something similar, but uses npm_config_argv that contains the original arguments from the install/uninstall command, if the package name is in the argument list the reshim is run

@lukaselmer lukaselmer force-pushed the feature/reshim-only-when-needed branch from d35af84 to ef5effb Compare January 25, 2021 17:02
@lukaselmer
Copy link
Contributor Author

@augustobmoura I resolved the merge conflict, and I like #197 👍

@augustobmoura
Copy link
Member

After changing the auto reshim mechanism in #232, this PR is probably deprecated. Thanks for the work anyways @lukaselmer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants