Installing dependencies with pnpm instead of git submodules

[PaulRBerg]

See my discussion with Drake Evans on Twitter:

https://twitter.com/DrakeEvansV1/status/1659612843849793573

[andreivladbrg]

That's interesting.

How would the remappings and the recursive submodules work? For example in our v2-periphery we import the prb-math types from v2-core/src/types/Math.sol and if we don't install the dependencies recursively we would be unable to utilize them.

Is there going to affect in any way the installation speed?

[PaulRBerg]

Remappings are easy to solve - we just define allow_paths in foundry.toml, see this example:

https://github.com/clabby/example-pnpm-forge-monorepo/blob/dd2a21d0545f7818597a6b4a7bb5611386153dae/packages/contracts/foundry.toml

import the prb-math types from v2-core/src/types/Math.sol

PRBMath is a peer dependency of V2 Core so users get a copy of the @prb/math package automatically.

Is there going to affect in any way the installation speed?

Installation times would be faster, actually, because Pnpm hoists dependencies (i.e. it does not install them multiple times, as git submodules do).

[PaulRBerg]

@andreivladbrg could you explore the possibility of ditching submodules in favor of Node.js dependencies?

[andreivladbrg]

I worked on this today (see here), it seems to be possible and in might be a good choice to switch to pnpm, at least in terms of time spent installing dependencies:

We have to test it in practice to see how would it work for integrators installing our repo. Will see how it goes on v2-periphery

Also, prb-math needs to be updated, because of the following files. They must be updated to hard-coded test-utils in src, instead of importing them from ../../test/utils/ path, test is not published in the package

[PaulRBerg]

For future reference: we ended up including the test/utils directory in the npm package:

PaulRBerg/prb-math#211

test is not published in the package

test/utils will be published in v4.0.2

[andreivladbrg]

Context

Using git submodules has started to be a pain in the ass because of the installation times.

I've been working on this, and it's a more complex idea than expected.

The installation process for @sablier/v2-core, both for the project itself and for external projects installing it, would be much faster. However, there are some edge cases.

External projects can be either foundry or hardhat based. I am not considering brownie here.

Foundry

If the external project is foundry-based, it can be installed via:

1. yarn add @sablier/v2-core
2. pnpm add @sablier/v2-core
3. forge install sablier-labs/v2-periphery

Any of these versions would require the project to add the following to their remappings file:

@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/
@prb/math/=node_modules/@prb/math/

Third party dependencies

By third-party dependencies, I mean the dependencies of the package that's being installed.

For 1. no additional installation is required because it will install its dependencies in node_modules.

The json file would look like this:

  "dependencies": {
    "@sablier/v2-core": "x.y.z",
   }

For 2. and 3. they would also need to manually install @prb/math/ and @openzeppelin/contracts/. For 2, it's necessary because of the way pnpm works with its symlinks. The third-party dependencies are placed in node_modules/.pnpm.

The json file would look like this:

  "dependencies": {
    "@openzeppelin/contracts": "x.y.z",
    "@prb/math": "x.y.z",
    "@sablier/v2-core": "x.y.z",
   }

Import and usage:

import { ISablierV2Lockup } from "@sablier/v2-core/src/interfaces/ISablierV2Lockup.sol";
import { Broker, LockupDynamic, LockupLinear } from "@sablier/v2-core/src/types/DataTypes.sol";

Hardhat

If the external project is hardhat-based, it can be installed via:

1. yarn add @sablier/v2-core
2. pnpm add @sablier/v2-core

No other remappings configuration is required.

Third-party dependencies

The same as foundry's 1 and 2.

Import and usage:

import { ISablierV2Lockup } from "@sablier/v2-core/src/interfaces/ISablierV2Lockup.sol";
import { Broker, LockupDynamic, LockupLinear } from "@sablier/v2-core/src/types/DataTypes.sol";

Question

Should we "force" our integrators that use foundry, which already have installed openzeppelin to add a new remapping to their file? IMO yes, we should recommend NPM installations in favor of git submodules and we should make a best practice for our remappings style.

Given the requirement of manually installing third-party dependencies for pnpm, should we switch to yarn? I'm aware that it's slightly slower.

Conclusions

In case I am not missing something, it's possible to completely switch to NPM packages from git submodules, and in my opinion, it looks like the correct decision.

What do you think?

[PaulRBerg]

@andreivladbrg can you please answer my question above:

Surely there must be a way to force Pnpm to not put the dependencies under node_modules/.pnpm without asking users to manually install those packages?

For maximum clarity, I am referring to integrators, not to us.

[PaulRBerg]

It would be worth asking the Pnpm maintainers for help - telling them that we would like to continue using Pnpm instead of Yarn:

https://github.com/orgs/pnpm/discussions

[PaulRBerg]

Closing as we have implemented this proposal in #734.