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

[FEATURE] new install protocol file+pack #1333

Closed
RecuencoJones opened this issue May 21, 2020 · 4 comments
Closed

[FEATURE] new install protocol file+pack #1333

RecuencoJones opened this issue May 21, 2020 · 4 comments

Comments

@RecuencoJones
Copy link

RecuencoJones commented May 21, 2020

What / Why

Currently it's possible to install packages from a folder, which is quite nice for quickly prototyping of monorepos, but becomes problematic with linking dependencies and binaries when things progressively grow and build processes are required in between.

The request to have a protocol file+pack would be pretty much the same, except it triggers npm pack on the directory prior to linking it to the parent.

This would allow hooking in with a prepack script to build/bundle the package before creating tar file.

As an example, we could have a repository like:

- app
- client
- shared
--- config (@app/config)
--- core (@app/core)
--- http (@app/http)
- tools
--- eslint (@app-tools/eslint)
--- tsconfig (@app-tools/tsconfig)

With a dependency graph similar to this:

- app
--- @app/config
--- @app/core
--- @app/http
----- @app/core (deduped)
--- @app-tools/eslint (dev)
--- @app-tools/tsconfig (dev)
- client
--- @app/config
--- @app/core
--- @app-tools/eslint (dev)
--- @app-tools/tsconfig (dev)
- @app/http
--- @app/core
--- @app-tools/eslint (dev)
--- @app-tools/tsconfig (dev)
- @app/core
--- @app-tools/eslint (dev)
--- @app-tools/tsconfig (dev)

Being able to pack @app/core before it being installed in app, client or @app/http would compile sources to generate dist folder while also allowing the tgz package to be installed in app, client or @app/http with just runtime dependencies as usual in npm install flow.

This may trigger multiple builds on packages which are repeated/shared between other dependencies. This could be solved by creating a dependency graph, keeping track of which packages were already built and building from leafs up.

It also means every single package is installable in isolation by default! No need for lerna bootstrap nor installing and hoisting all packages dependencies when just a few are required, which potentially improves CI times and splitting of builds.

How

Not sure how the exact implementation for this would be, but given the following packages:

{
  "name": "package-a",
  "version": "1.0.0",
  "dependencies": {
    "package-b": "file+pack:../package-b"
  }
}
{
  "name": "package-b",
  "version": "1.0.0",
  "main": "./index.js",
  "scripts": {
    "prepack": "tsc"
  },
  "devDependencies": {
    "typescript": "^3"
  }
}

When resolving file+pack the following will happen:

package-b has prepack script?
    npm install on package-b directory (dependencies + devDependencies)
npm pack
install generated package-b-1.0.0.tgz on package-a

Related issues

#529 (comment)

#702

#459 (comment)

Possible caveats

  • Packages using file+pack are usually intended for local usage / fixed versioning as part of monorepos and usually not meant to be published nor shared around
  • How to keep integrity with package-locks? Maybe packages using this protocol should use a different strategy.
@isaacs
Copy link
Contributor

isaacs commented May 22, 2020

This should be an rfc for sure. It's a good idea, and one that might make my personal day to day work a bit easier. If so, I'd be eager to get it implemented 😂

Right now the workaround is to pack the dir, and then install the packed tarball. That of course works, but it's an extra step. The advantage there is that the installed dep only changes when the artifact is re-generated. The disadvantage, obviously, is the extra step.

Some other thoughts:

  • It seems like at least part of the need here stems from the fact that you want the prepare script to be run in the linked dep. Could we get part of the way with a flag to just tell npm to do that? Ie, npm install --rebuild-links or something? That flag would also be respected by default by the rebuild command, so you could do npm rebuild --rebuild-links when you know something's changed, and even set up a watcher to do that while in development. (This would be considerably faster than creating and unpacking a tarball.)

  • If the bytes don't change, the integrity won't change. (As of npm v7, the integrity of the gzipped tarball will also be consistent across operating systems, because we strip out the annoying useless hard-coded gzip OS header.) We may need to have a slightly different strategy for reifying these deps though. A matching integrity in the lockfile and a newly generated artifact will show us that nothing has changed, but a changed integrity means that we need to replace it. So, it's less an expectation, per se, and more a way to just track what we already have and avoid extra work, maybe? I would be surprised if I change something in my locally packed dep, and then the install fails with an EINTEGRITY.

  • For publishing and sharing, I see this as roughly equivalent to tarballs or symlink deps, which we do allow you to publish with today, and has for a long time. It's not super portable, of course, but it doesn't seem to cause serious problems, and people using this feature tend to understand the impact. (Or if they don't, they learn quickly when their users complain.)

@isaacs
Copy link
Contributor

isaacs commented May 22, 2020

Also, spelling of the protocol needs to be bikeshedded. I don't know off the top of my head any reason to prefer file+pack: vs pack: vs pack+file: etc., but I know we do test the resolved string against /^file:/ in a few places to see whether it's a local dep, so all those code paths will have to be examined.

We might also explore something like pack:file:/path/to/folder, where pack: can be a prefix applied to any arbitrary spec? Then you could do something like pack:express@latest. It'd be extraneous in the case of git deps, where we already have to repack for consistency, but I'm wondering if there might be cases where it's worthwhile to re-generate a tarball that you get from a url or registry? Like if it is a url like https://my-company/module/src.tgz tarball of the raw src code that has to be built?

@RecuencoJones
Copy link
Author

Hi @isaacs, thanks a lot for the quick response and insightful feedback!

This should be an rfc for sure. It's a good idea, and one that might make my personal day to day work a bit easier. If so, I'd be eager to get it implemented 😂

Didn't know what would the exact process, should I create the pull request to keep the discussion there?

  • It seems like at least part of the need here stems from the fact that you want the prepare script to be run in the linked dep. Could we get part of the way with a flag to just tell npm to do that? Ie, npm install --rebuild-links or something? That flag would also be respected by default by the rebuild command, so you could do npm rebuild --rebuild-links when you know something's changed, and even set up a watcher to do that while in development. (This would be considerably faster than creating and unpacking a tarball.)

Completely agree! I was just thinking about this some minutes ago 😄
Using a flag + rebuild command looks quite a good solution for me, you could actually target a single dependency to rebuild, which would be superb.

  • For publishing and sharing, I see this as roughly equivalent to tarballs or symlink deps, which we do allow you to publish with today, and has for a long time. It's not super portable, of course, but it doesn't seem to cause serious problems, and people using this feature tend to understand the impact. (Or if they don't, they learn quickly when their users complain.)

That's a reasonable "caveat" then, I wouldn't like adding logic to publishing where npm would need to check dependency tree to refuse in case of such a dependency.

Also, spelling of the protocol needs to be bikeshedded. I don't know off the top of my head any reason to prefer file+pack: vs pack: vs pack+file: etc., but I know we do test the resolved string against /^file:/ in a few places to see whether it's a local dep, so all those code paths will have to be examined.

I thought about file+pack, mainly because the syntax I was using resembles or downright equals file: dependencies. I also liked having pack: implicitly there as it's easy to map it to the npm script that will be run, where you can hook with the prepack script. In any case I trust you will know better what would be the appropriate option!

We might also explore something like pack:file:/path/to/folder, where pack: can be a prefix applied to any arbitrary spec? Then you could do something like pack:express@latest. It'd be extraneous in the case of git deps, where we already have to repack for consistency, but I'm wondering if there might be cases where it's worthwhile to re-generate a tarball that you get from a url or registry? Like if it is a url like https://my-company/module/src.tgz tarball of the raw src code that has to be built?

I didn't think about this – and right now I can't think of a use case –, but I do like the idea of composable schemas.

@RecuencoJones
Copy link
Author

@isaacs created RFC pull request npm/rfcs#150 to continue discussion there!

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