Skip to content
This repository has been archived by the owner on Dec 6, 2023. It is now read-only.

Direct support for .nvmrc? #17

Closed
danawoodman opened this issue Jul 9, 2015 · 11 comments
Closed

Direct support for .nvmrc? #17

danawoodman opened this issue Jul 9, 2015 · 11 comments

Comments

@danawoodman
Copy link

Is it possible for avn to directly support the .nvmrc file I already have in my project?

Perhaps some logic could be incorporated to do an OR operation when searching (e.g. '.node-version' || '.nvmrc'). Having to duplicate that file is unfortunate because then both have to be in sync.

I'm currently symlinking the file but would love to not have to do that.

Thanks!

@danawoodman
Copy link
Author

@ljharb
Copy link

ljharb commented Jul 9, 2015

Since avn supports multiple node version managers, and node-version isn't a standard, but simply an avn format, I'd expect it to support any manager-specific format that's available.

@wbyoung
Copy link
Owner

wbyoung commented Jul 13, 2015

I think I agree, this makes sense to support. I'm hesitating slightly on this for a few reasons:

  1. I view the .nvmrc file as serving a slightly different purpose. Since nvm doesn't do automatic version switching, the intent of this file feels to me like it is for configuring nvm, not for indicating automatic changes. By keeping the files separate, intent may be a little more explicit.
    There's also a functional difference since nvm looks both in the current directory as well as any parent directory. Should avn then perform a switch with the same logic? Probably not, because then with a ~/.nvmrc file, all directory changes inside your home folder would trigger a switch (not to mention the overhead of searching the directory ancestry for every cd).
    Having avn only change directories when the .nvmrc is in the current directory would make more sense for avn, but then the file works slightly differently between these two tools.
  2. The architecture of avn hides away the details of which version switcher it's working with via plugins & this would probably require adding nvm specific logic to the main project (for speed reasons). This reasoning is not a good reason since it's all about implementation and not about functionality, but I wanted to add my thoughts here. It also may be possible to address this.

.node-version is somewhat specific to avn, but nodenev does uses the same file to achieve the same sort of thing as avn. This can actually be used as an argument against _#_1 because nodeenv searches all parent directories & also does not perform automatic switching upon cd (based on my limited knowledge of it).

I'm curious about your thoughts on _#_1, @ljharb. Can you see potential confusion among users if avn only looked in the current directory for .nvmrc files?

@wbyoung
Copy link
Owner

wbyoung commented Jul 14, 2015

Implementation in 3d74623 shows that _#_2 from above is actually not a concern.

One side effect of the current implementation is that an .nvmrc file could trigger a version change via avn-n and n.

@ljharb
Copy link

ljharb commented Jul 14, 2015

I don't understand why ~/.nvmrc would be a problem - it would only trigger a switch when cd-ing across the boundary of the ~ subtree, but not when staying within the subtree.

I think that any .foo file that doesn't look upwards is likely to cause confusion, as that's the most typical behavior I've seen. The inconsistency is another reason not to do it.

For what it's worth, nvm doesn't do automatic version switching when within the same shell, but it definitely does when opening up a new shell - so I'd say your # 1 from above isn't much of a concern.

I also don't think that .nvmrc should have any impact on n or nave.

@wbyoung
Copy link
Owner

wbyoung commented Jul 14, 2015

@ljharb it wouldn't cause a problem for which version should be activated, but it would cause a slight performance problem as we'd be checking more directories for the presence of a file.

A quick test reveals that 30 additional file checks added to the cd hook makes little difference on my machine (SSD). When performing these checks on spinning disks or network mounted volumes, that performance impact could change. I'm not sure it seems right to patch cd and add N file checks.

The bigger performance issue is the way that things are architected right now, a node script is run when the .node-version (or .nvmrc) is found to activate the proper version. This script can be quite slow, so using cd would possibly always be slow. This could probably be fixed with some additional changes within avn.

@wbyoung
Copy link
Owner

wbyoung commented Jul 14, 2015

Dotfile Thoughts:

I think the .foo files looking upward seem somewhat case dependent. There are some cases like jshint/jshint#833 and jshint adding an extends option that probably would have been better off from the start with this type of behavior. On the other hand, for JSHint it makes sense for a project to be able to specify rules and know that a ~/.jshintrc won't be setting any options by default. JSCS seems to have the same issues as JSHint jscs-dev/node-jscs#1106, jscs-dev/node-jscs#559. In the case of .gitignore, git can determine the project root because it knows where the .git directory is & can stop there. .vimrc is read from the current directory, overriding ~/.vimrc, but does not read from parent directories. git does the same sort of thing with .git/config & ~/.gitconfig.

These cases fall into two different categories:

  1. Per-project configuration (shared between collaborators)
  2. Per-user configuration (shared across one user's projects)

Per-project configuration can only look upward by default if it knows where the project root is (passing the project level has it cross into per-user). That's why JSHint and JSCS are better served with an extends option. I believe their intention to be per-project configuration. (They do support per-user configuration as well via ~/.jshintrc, but a project level file completely overwrites the user config.)

Per-user configuration can look anywhere on the machine & it's still a configuration for the user. A ~/.vimrc usually includes configuration for the user while a .vimrc file in the working directory is used for project level configuration. Unlike .jshintrc, these options are merged. The .gitconfig is exactly the same.

With .gitignore files, you get per-directory configuration on a project level basis, but as mentioned before, git knows where the project root is. It stops there.

So the main question, is automatic version switching something that is per-project or per-user.

If it's per-project, then avn should not look in parent directories as it will not reliably be able to determine where the project root is.

Perhaps, though, it's the responsibility of the user to make this decision. They install the files where they want & we don't worry — just always assume that traversing outside of a project is acceptable.

~
├── .node-version: iojs-1.0
└── projects
   ├── .node-version: 0.12
   ├── first
       ├── .git
       ├── .node-version: 0.10
       ├── package.json
       └── src
   ├── second
       ├── .git
       ├── .node-version: 0.12.7
       ├── package.json
       └── lib
   └── thrid
       ├── .git
       ├── package.json
       └── lib

What I don't like from above is that the ~/projects/.node-version file can easily create a situation where you end up with unexpected behavior. Moving ~/projects/third to a new location, ~/archived-projects/thrid that lacks a .node-version file changes the way that the third project works. Of course, that's behavior that should be obvious based on how the user set things up. To avoid this behavior, one would simply avoid creating the user-level config, ~/projects/.node-version.

wbyoung added a commit that referenced this issue Jul 15, 2015
This includes the following changes:

- Plugins can provide a load.sh file
- Custom version files are supported via plugins adding to `__avn_files`
- Parent directories are searched for version files.
- Configuration will not be updated when active version file has not changed.
- Output includes chosen version file when it is not `./.node-version`.
@wbyoung
Copy link
Owner

wbyoung commented Jul 15, 2015

@danawoodman I've got something pushed on some branches (10e0ae9 and 48445caa) if you'd like to give it a try.

Do the following, then restart your shell:

rm -r ~/.avn
npm i -g wbyoung/avn#version-files wbyoung/avn-nvm#version-files
avn setup

I'd love to know whether it works well for you or not.

@wbyoung
Copy link
Owner

wbyoung commented Jul 18, 2015

This is now available in avn 0.2 and avn-nvm 0.2. Run the following, then restart your shell:

npm i -g avn avn-nvm
avn setup

@danawoodman
Copy link
Author

Awesome, thanks @wbyoung that's working great! Only thing I got caught up on was it seems avn creates a .bash_profile file which caused by .profile not to load. Moving the init code to .profile fixed it

@wbyoung
Copy link
Owner

wbyoung commented Jul 28, 2015

@danawoodman I'd be happy to accept a pull request that accounts properly for all configurations of all shell types and config files as we'd expect them to be appropriately used across different platforms. Right now avn simply assumes the use of .bash_profile and/or .zshrc. Obviously this does not cover all cases properly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants