Fix Bug when node_modules are outside of project

[ultrox]

I'm not sure is this intended or not, but I think its more stable then using "app-root-path".

Said package fails to find package json, when node_modules are placed outside of project in any way, including when "node_modules" are symlinked in the project.

process.cwd() works better because it will always be actually root project in this case.

[codecov]

Codecov Report

Merging #477 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #477      +/-   ##
==========================================
+ Coverage   98.74%   98.75%   +<.01%     
==========================================
  Files          12       12              
  Lines         239      240       +1     
  Branches       28       28              
==========================================
+ Hits          236      237       +1     
  Misses          3        3

Impacted Files	Coverage Δ
src/findBin.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d52e074...50657b4. Read the comment docs.

[sudo-suhas]

This PR is not acceptable as it has unnecessary and incorrect changes to the documentation and package.json. We only use the package.json resolved using app-root-path to provide helpful error messages when findBin fails. So instead we could just make the package.json resolution error tolerant.

[ultrox]

@sudo-suhas that's fine. Btw documentation changes are on my fork I use for the project, I didn't' intend it for this pr.

[okonet]

The idea behind using app-root-path is to be able to locate the root package.json even when doing a commit from a sub-directory without it. process.cwd will not take this into account AFAIK.

when node_modules are placed outside

I can't imagine such case since package.json and node_modules are tightly coupled.

[sudo-suhas]

Taking the case of lint-staged being invoked from the pre-commit hook, husky executes npm run -s precommit which means the process.cwd() matches the package.json location. I feel that since the package.json resolution only affects error message being shown in case of an unexpected error in findBin, we can afford to rely on process.cwd() since it works for majority of cases. I already tested this by tweaking findBin in the example repo https://github.com/sudo-suhas/lint-staged-multi-pkg.

[sudo-suhas]

I added a console.log for the package.json which was loaded using require(`${process.cwd()}/package.json`)

lint-staged-multi-pkg/packages/say-bye on  master [+] is 📦 v1.0.0 via ⬢ v10.6.0 
➜ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	modified:   ../../lerna.json
	modified:   .eslintrc.yml
	modified:   lib/index.js
	modified:   package.json
	modified:   yarn.lock
	modified:   ../say-hi/.eslintrc.yml
	modified:   ../say-hi/package.json
	modified:   ../say-hi/src/index.js
	modified:   ../say-hi/yarn.lock


lint-staged-multi-pkg/packages/say-bye on  master [+] is 📦 v1.0.0 via ⬢ v10.6.0 
➜ git commit
husky > npm run -s precommit (node v10.6.0)

lerna info version 2.5.1
say-bye: { name: 'say-bye',
say-bye:   version: '1.0.0',
say-bye:   description: 'Just say Bye!',
say-bye:   main: 'lib/index.js',
say-bye:   scripts: { precommit: 'lint-staged', test: 'echo "LOL" && exit 1' },
say-bye:   author: 'Suhas Karanth <sudo.suhas@gmail.com>',
say-bye:   license: 'MIT',
say-bye:   devDependencies: { eslint: '^5.3.0', 'lint-staged': '^7.2.0' } }
say-bye: Running tasks for lib/*.js [started]
say-bye: eslint --fix [started]
say-bye: eslint --fix [completed]
say-bye: git add [started]
say-bye: git add [completed]
say-bye: Running tasks for lib/*.js [completed]
say-bye: 
say-hi: Running tasks for src/*.js [started]
say-hi: eslint --fix [started]
say-hi: eslint --fix [completed]
say-hi: git add [started]
say-hi: git add [completed]
say-hi: Running tasks for src/*.js [completed]
say-hi: 
lerna success run Ran npm script 'precommit' in packages:
lerna success - say-bye
lerna success - say-hi
hint: Waiting for your editor to close the file... Gtk-Message: 14:43:43.028: Failed to load module "canberra-gtk-module"
Aborting commit due to empty commit message.

[sudo-suhas]

Also, we use process.cwd() with npm-which as well.

npm-which will find executables relative to the cwd you supply.

Since we haven't faced any issue with binary resolution, I am reasonably confident with using process.cwd().

[okonet]

Okay. Then ship it :)

[Vanuan]

@okonet

The idea behind using app-root-path is to be able to locate the root package.json even when doing a commit from a sub-directory without it. process.cwd will not take this into account AFAIK.

In that case it sounds more reasonable to look for the topmost .git folder:

git rev-parse --show-toplevel

[Vanuan]

when node_modules are placed outside

I can't imagine such case since package.json and node_modules are tightly coupled.

They aren't:

# .yarnrc file in project root
--modules-folder /path/to/node_modules/elsewhere

[Vanuan]

@sudo-suhas @okonet What's the status of this PR?

[okonet]

@Vanuan would you mind propose a working solution and file a PR? As @sudo-suhas we can't merge this PR as is and it was closed by the author. Alternatively, I'd still be happy to accept @ultrox work but we'd need to clean things up and add related tests for this edge case.

[Vanuan]

I've upgraded to latest and the issue appears to be fixed. Not sure why since this PR is not merged