search

lukechilds / zsh-nvm

Zsh plugin for installing, updating and loading nvm
MIT License
2.22k stars 112 forks source link

Make sure lazy loading works with future updates to nvm #2

Closed lukechilds closed 8 years ago

lukechilds commented 8 years ago

I've just added added lazy loading to zsh-nvm https://github.com/lukechilds/zsh-nvm#lazy-loading. I've tested and it all seems to be working well.

@ljharb I'm sure I remember reading an issue on nvm about lazy loading where someone had posted a method similar to this and you said that it would break with future changes you were planning to make to nvm regarding lts or something? Sorry, I've been searching but I can't find the exact issue now.

I've tried to be as generic as I can looking for binaries, checking everything in the versions folder not just node. I'm sure you're a very busy man but I'd really appreciate it if you could cast your eyes over this and give me a heads up if you think any changes you're gonna make to nvm will break this.

Juicy bits here: https://github.com/lukechilds/zsh-nvm/blob/2a9ee9e10f3aa9fc428bcb05406aec8396c802ac/zsh-nvm.plugin.zsh#L49-L67

No worries if you're too busy!

ljharb commented 8 years ago

Basically, hardcoding the directory structure is not a great idea - your current example won't currently work for node versions under 0.12, for example, and when I introduce RCs and nightlies, it will likely not work as you expect.

lukechilds commented 8 years ago

Thanks for your input 👍

I can fix <0.12 by also checking "$NVM_DIR"/v0*/bin/*.

Where are you planning installing RCs and nightlies then, not versions/rc/ and versions/nightly/? Cos that would match the current glob.

ljharb commented 8 years ago

Actually I think my current plan will match the glob, since you're not trying to parse version numbers - you're just trying to locate binary names.

It might be a better approach to source nvm with --no-use (since it's the auto-using that's slow, not sourcing nvm itself), and then we can perhaps make nvm expose a list of binary names that you can use?

lukechilds commented 8 years ago

nvm --no-use is still a fair bit slower than my current lazy loading solution though:

% time (source $NVM_DIR/nvm.sh)
( source $NVM_DIR/nvm.sh; )  1.44s user 0.98s system 109% cpu 2.201 total

% time (source $NVM_DIR/nvm.sh --no-use)
( source $NVM_DIR/nvm.sh --no-use; )  0.02s user 0.03s system 97% cpu 0.058 total

% time (_zsh_nvm_lazy_load)
( _zsh_nvm_lazy_load; )  0.01s user 0.01s system 166% cpu 0.012 total

Plus I'd then have to call nvm to get the binaries and then loop over them to create the lazy lading functions on top of that 58ms. Not too sure how I'd handle loading node either. I could run nvm use default in the lazy loaders but I think that might then (maybe?) override .nvmrc settings. Currently just lazy loading all of nvm means that .nvmrc and stuff are just checked as normal.

If you're not planning to make changes to the directory structure I reckon I may as well stick with my current solution. Doesn't seem worth bloating nvm with a new function just for zsh-nvm which will make the lazy loading ~5x slower with no usability difference.

Make sense?

ljharb commented 8 years ago

Sure, that makes some sense. Although, I think 0.02s vs 0.01s is small enough that it's basically identical.

lukechilds commented 8 years ago

Total is the figure to compare though which is 58ms vs 12ms. And that 58ms is just loading nvm I'd still need to get binaries and create loader functions on top of that. The current 12ms does everything.

Thanks for looking over this, appreciate it 👍