inxilpro / node-app-root-path

Determine the root path to your project
MIT License
605 stars 29 forks source link

node_modules isn't cool anymore, what to do #29

Closed dominic-p closed 5 years ago

dominic-p commented 5 years ago

I just read about yarn's new plug n play install and it looks like npm might be pursuing something similar.

Since one of the main heuristics used here is the presence of node_modules, I could see this breaking if these new module resolution systems gain traction.

afzalah commented 5 years ago

I guess it's like doing the same thing over and over again, why yarn doesn't simply embed/use @verdaccio and support in their development instead.

inxilpro commented 5 years ago

Hm, yeah. It seems like we'll need new heuristics. Sadly, it looks like PnP will fundamentally break the way this library works…

Luckily, our fallback strategy of using require.main.filename should continue to work in most cases, but it's not ideal.

dominic-p commented 5 years ago

Yeah, that's true. I have played around a bit with require.main and it's definitely not bullet proof, especially when running tests.

inxilpro commented 5 years ago

Yeah, test-runners and anything else that inject themselves before your app will break that method. We may need to just check for those cases and rely on require.main more heavily, though…

arcanis commented 5 years ago

FYI with PnP it should just be a matter of doing this:

const pnp = process.versions.pnp
  ? require('pnpapi')
  : null;

const appRoot = pnp
  ? pnp.getPackageInformation(pnp.topLevel).packageLocation
  : null;
benface commented 5 years ago

This package seems to be checking for package.json instead of node_modules: https://github.com/sindresorhus/pkg-dir

Would that work? Is it too simple/naive and doesn't work in some cases?

dominic-p commented 5 years ago

Good suggestion @benface. I've played around with looking for package.json. It seems a bit less robust than node_modules in my testing.

For example, if you are in a path inside node_modules, you might assume your current directory is the root because it has a package.json file when really you want the parent directory (the one that has node_modules in it).

For example:

/some/path/project/node_modules/mocha/package.json
/some/path/project/package.json
benface commented 5 years ago

Oh, ok so it wouldn't work for task runners / test frameworks because those run from their own directory?

arcanis commented 5 years ago

Note that the Yarn cache is (in most cases for now) outside of the project folder. So if you simply traverse upwards until you find the top-most node_modules / package.json, the only one you'll find are your own.

For a perfect integration, I recommend my snippet above 🙂

Vanuan commented 5 years ago

Another thing to consider is that yarn allows to override node_modules location:

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

I'm disappointed. I thought app-root-path is doing something smarter than making assumptions about its own location in a filesystem.

Vanuan commented 5 years ago

Look here: https://github.com/okonet/lint-staged/pull/477

inxilpro commented 5 years ago

Thanks @arcanis — 2.2.0 adds pnp support.