Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Nested Local Dependency Installation (via hoisting?) #41

Open
eettaa opened this issue Jan 20, 2022 · 2 comments
Open

Enhancement: Nested Local Dependency Installation (via hoisting?) #41

eettaa opened this issue Jan 20, 2022 · 2 comments

Comments

@eettaa
Copy link

eettaa commented Jan 20, 2022

Hi! Thanks so much for this package, it seems to be the only public solution for actually copying local packages into their clients' node_modules folder. Super useful for testing. I'm actually planning to use it in a production monorepo + microservices layout where the microservice bundling requires all dependencies to be in a single directory subtree (no symlinks to a global util folder or something).

One limitation I've discovered is that nested local dependencies don't currently work. That is, if local package App depends on local package Lib that depends on local package Util, npx install-local path/to/Lib won't pull in Util as well. I imagine this is because running npm pack on Lib doesn't bundle the Util dependency and adding the Lib tarball to Client doesn't respect the (custom) localDependencies field in lib/package.json (or associated tarball).

Possible solutions:

  • Adding Util to Lib's bundledDependencies solves this problem but then makes running npm install in the Lib package fail with "No valid versions available for Util." This happens even if install-local is run in npm's preinstall hook, I assume because npm install cleans out all dependencies not in package-lock.json from node_modules/ , and package-lock.json isn't updated when install-local adds local dependencies.
  • Declaring Util as a peerDependency doesn't work because you would have to point to a tarball or external package, and one of the great things about this library is that it hides tarball creation.
  • My Solution: Manually hoist all localDependencies of Lib to App, then run npx install-local. Manual works for a small dependency tree but is super annoying as the trees grow. In particular it isn't as simple as copy-pasting- you need update all the relative paths to be correct w.r.t. your client package. Still, doable for small repos.

It seems like this would be relatively easy to do as part install-local --save, although we'd have to be careful about messy/invalid/circular dependency trees. But it would make this solution a little more fully fledged. npm would continue to beautifully handle all the non-local dependency resolution (hoisting shared versions, installing nested local versions, etc).

I'm super psyched to have discovered this package and would love to hear what people more familiar with the package think of this idea!

@nicojs
Copy link
Owner

nicojs commented Jan 21, 2022

Thanks for the kind words.

My Solution: Manually hoist all localDependencies of Lib to App

If I understand you correctly, then yes, this is the best solution. This is what I do for StrykerJS as well:

For example:

{
  "localDependencies": {
    "@stryker-mutator/api": "../../../packages/api",
    "@stryker-mutator/core": "../../../packages/core",
    "@stryker-mutator/instrumenter": "../../../packages/instrumenter",
    "@stryker-mutator/karma-runner": "../../../packages/karma-runner",
    "@stryker-mutator/util": "../../../packages/util"
  }
}

Where @stryker-mutator/core and @stryker-mutator/karma-runner are the only direct dependencies (rest are transient dependencies).

Is this what you mean?

I think it is very difficult to improve this because this is how npm works. I already think it is quite remarkable that this approach works in the first place, it's a pretty exotic use case.

@eettaa
Copy link
Author

eettaa commented Jan 21, 2022

Where @stryker-mutator/core and @stryker-mutator/karma-runner are the only direct dependencies (rest are transient dependencies).
Is this what you mean?

Yup, exactly! :)

I can't claim to be an npm expert, but I was wondering if install-local could solve this. I imagine the install-local algorithm becoming somewhat recursive, like:

totalTDeps = []
for each dep in package.json localDependencies:
- tdeps = the localDependencies of dep
- for each tdep in tdeps:
  -  if normalizePath(tdep) not in totalTDeps:
    -  add tdep relative path to totalTDeps
    - (recursively continue this for tdep)
...
then, after dependency tree traversal, back in the package where install-local was run:
- for each dep in localDependencies, do what install-local already does
- update a new field in package.json called transitiveLocalDependenciesDONOTMANUALLYEDIT to contain totalTDeps
- run existing install-local algorithm for each of these as well.

normalizePath() would output the path of the tdep relative to the package where install-local is run, which both removes the most tedious part of the manual hoisting (changing the paths) and solves dep disambiguation (what if two libs have same util is local dep under different names or something).

I suggest the separate transitiveLocalDependencies field because this could be cleaned/reinstalled more brazenly than entries in localDependencies
Obviously this is rough- it's just meant to give the general overview. But I think it would be totally doable (and in the purview of) install-local, not npm (?)

You might have more insight into the difficulties than I do. From my side this is the last remaining functionality to make this library a full local-build solution on top of npm, which would be awesome. It still amazes me that this logic isn't native to npm, it's super useful :)

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

No branches or pull requests

2 participants