https://pantsbuild.org/ logo
Join Slack
Powered by
<@U03JDB8MCE7> still messing around with `package_...
# development
l
@worried-painter-31382 still messing around with 
package_json
here. I noticed that the 
node_build_script
target does not have all the 
node_modules
of the package’s pnpm workspace dependencies copied to its sandbox. Is that expected?
w
Yes, I believe only its dependencies are copied
l
Using 
keep-sandboxes=always
I can see that the sandbox for the install step have all the node_modules of the other workspaces installed.
But then they’re not there in the build step sandbox. That makes it impossible to build without all the other node_modules
w
I cant check this today but the "build_script" digest is a superset of the installation digest. If they arent the 
InstalledNodePackage
rule is doing something wrong
The intention was to not copy node_modules for unrelated workspaces, but maybe it filters things it shouldnt
l
Might it be because since I’m using typescript pants can’t infer the dependencies from code?
w
The intention was to not copy node_modules for unrelated workspaces
This because the amount of IO is obscene as is
Might it be because since I’m using typescript pants can’t infer the dependencies from code?
I mean, possible? I'm not confident it works in complex workspace setups, my org uses this feature sparingly
and all three package managers construct node_modules in workspaces differently
l
True! In the case of 
pnpm
it’s creating a node_modules for each package in the workspace that the one we’re filtering has dependencies on. So if pkg1 depends on pkg2 and pkg3, when we run 
pnpm install --filter=pkg1
it’s installing all three of them and symlinking the folders where needed.
But since you told me pants doesn’t know anything about the workspace dependency via 
package.json
just via code imports I’m guessing that is where the issue lies for me
Alright so in 
ìnstall_node_packages_for_address
rule, the digest only includes 
project_env.node_modules_directories
. So it will never pass on the other node_modules for its dependencies. I think there’s a use case for copying all installed node_modules of dependencies, in particular for monorepos that share non-compiled libs between apps
w
You sure about that? The install is recursive for all dependencies (which might be really bad for pnpm if it always installs everything)
but I'd expect the previous node_modules to be part of the input digests of the new install, i.e carry over when the new installs node_modules is merged with the "install_input_digest"
I can investigate with code further probably sunday and provide better answers instead of guesses/questions
l
If I add 
pkg2:package-json
as a dependency to 
pkg1
, it does install it. However, it will fail if there are common packages in a “package.json” in a parent folder saying that it can’t merge folders due to duplcate files. In my example I have a package.json in the root that has eslint as a dependency. Since both 
pnpm installs
would create node_modules/.bin/eslint in the root, the merge will fail
w
okey, yes so this
which might be really bad for pnpm if it always installs everything
is very true then. I think maybe the approach is poor to begin with for pnpm, and possibly not worth the complexity in npm and yarns case
it's much simpler to install the entire workspace once as a baseline, and optimize by subsetting based on deps. That rules out the bad merge behaviour as well 🤔
l
Just checked, even if I don’t have the parent package.json, it will still complain about duplicate entries in 
node_modules/.bin
bc both of the packages are installing eslint.
it’s much simpler to install the entire workspace once as a baseline, and optimize by subsetting based on deps. That rules out the bad merge behaviour as well 🤔
Technically, if we set up 
node-linker=hoisted
pnpm will install a “flat” node_modules meaning all of the packages would install in the same folder all of the dependencies that are common between them and only have a local node_modules for dependencies that are exclusive to that package or that have version conflicts.
In this case we can remove the root ‘node_modules’ from 
project_env.node_modules_directories
(or guarantee it’s only used once)
4 Views
Open in Slack
PreviousNext
Powered by