Migrate to pnpm

Hello all,

In a previous work, I introduced a mechanism to make it easier for Webjar Maven projects to build their resources using npm.

I’m in the process of refining this mechanism to use pnpm instead.
The two main benefits are

  • Workspace | pnpm - allowing to group all packages in a common workspace, making it easy to have packages depending on each other
  • Catalogs | pnpm - allowing to share package dependencies uniformly in a workspace.

This is also an important step to be able to move packages from cristal to xwiki-platform, which are already based on pnpm workspaces.

To track this work, see XCOMMONS-3464 and the following PRs:

Commons PR description

I’m duplicating and expanding the content of the PR description here to ease the reading.

npm is replaced by pnpm in webjar-node.
This allows us to have all the webjar-node modules as a pnpm workspace (see XCOMMONS-3464: Migrate to pnpm by manuelleduc · Pull Request #4721 · xwiki/xwiki-platform · GitHub).
This also allows us to use pnpm catalogs which is a recent mechanism to centralize dependency versions instead of duplicating them in all packages.
We experienced it in Cristal first and proved successful and greatly simplified npm dependencies upgrades.
This is also well supported by renovate.
An additional benefit is that the lock file is now unique at the root of the repository instead of one for each module.

Because of having all the packages in the pnpm workspace, I stopped moving the JavaScript sources to target before starting the build.
This is important because when the sources are moved, they are not part of the workspace anymore (or with a duplicated package id), and this is breaking dependency resolution.
The main cons I’d like to highlight is that since the JavaScript build is now done “inplace”, a dist and a node_modules are created during the build (they are git ignore, but are still there).

To the best of my knowledge, this approach is the best option we have.
Having all the build artifacts in the target directories while preserving a sound pnpm workspace would imply tweaking versioned files (e.g., pnpm-workspace.yml at build time), which seems worse than producing git ignored directories at build time.

Conclusion

This is a breaking change for webjar-node. I consider this ok since this was introduced in June this year and can be considered as unstable. To my knowledge, only the Cristal Integration extension is using it outside XS.

The production of git ignored directories outside the Maven target directory is unusual, and I’d like to validate that this is not creating unforeseen issues.

Thanks!

Is the lock file from the root of the repository (commons / platform) updated when I modify and build only a submodule (say xwiki-platform-blocknote-webjar)? If that is the case then I find it unexpected that building a submodule leads to changes outside the working directory. If it’s not the case, then how do we update the root lock file?

Moreover, isn’t a lock file needed / useful inside the WebJar generated for a submodule?

These two folders are going to be ignored globally, I suppose, which means modules not using pnpm will not be able to use them. It’s not a problem with node_modules which is very specific, but dist is quite generic. I hope we won’t have the need to use it.

Are these 2 folders deleted by mvn clean? Otherwise, isn’t there a risk to build using old / left over code, especially when switching Git branches?

I’m +1 to move to pnpm, but I don’t like building the JavaScript code inplace, inside the source folder. I trust you that there is no better / cleaner option, so I’ll have to leave with it.

Thanks,
Marius

Only when the list of dependencies (e.g., add/remove a dependency, upgrade a dependency) is changed.
Nothing is changed outside the module at build time.

It is not; pnpm resolves the workspace even when called from Maven.
What you couldn’t do is to only clone a sub-directory of the repository. But this constraint is not new, and the same is true for Maven.

I realize they are already ignored since April on master.
They currently are only here to avoid pushing those accidentally but are not strictly needed.
The node_modules/ will become mandatory.
For the dist/, I thought about it and see two options:

  1. the dist directory itself could actually be in the target directory. I’ll try this in my local PoC to see if there is no unexpected consequences.
  2. if the first option does not work, we could freely choose another name for dist, less likely to cause conflicts with the rest of the codebase. For instance: target or target_node.

That’s a good point! I’ll check if I can tweak the maven clean plugin configuration to also remove the directories mentioned above, in addition to the usual target directory.

1 Like

So regarding moving node_modules to another location.
A modulesDir setting exists in pnpm-workspace.yaml but it is poorly supported currently.
I’ve modulesDir to ../../../target/node_modules and the node_modules was correctly located in the target which is bit.
But if a dev dependency provides a binary (e.g., Vite), calling pnpm exec vite will not be able to find the binary when a custom modulesDir value is defined.
In conclusion, this is not an option currently (but might become one when this setting is better supported).

Regarding the dist directory, it’s actually easy to configure vite to output outside the current directory by adding outDir: '../../../target/node-dist'.
But this solution is fine only for packages that are not meant to be published on a npm registry afterward (so basically only for module of type webjar-node).
Following this PR, I’ll open another PR where some parts of Cristal are moved to xwiki-platform.
This is likely to create some other challenges of this kind, so I suggest we discuss them separately.

Done with Loading...