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

fix(resolve-dependencies): don't fail when dependency tree doesn't contain parent package #5614

Closed
wants to merge 1 commit into from

Conversation

szilarddoro
Copy link

@szilarddoro szilarddoro commented Nov 10, 2022

This PR fixes the problem mentioned in #5327.

We started experiencing problems with pnpm install when migrating our dashboard to our monorepo. Although the dashboard project was using yarn originally, I decided to migrate to pnpm in the standalone repository first. Everything was working fine there. It only started breaking when the project was moved to the monorepo.

P.S: The problem can be reproduced in our repository by removing pnpm-lock.yaml and running pnpm install.

@zkochan
Copy link
Member

zkochan commented Nov 10, 2022

Thanks, we need to add a test for this. I don't want this to regress in the future.

@szilarddoro
Copy link
Author

Thanks, we need to add a test for this. I don't want this to regress in the future.

Yes. I'll try to prepare something for this soon, but couldn't find the exact scenario when this happens, so I need to keep digging a bit.

@szilarddoro
Copy link
Author

@zkochan I've tried to reproduce this scenario in my test cases through projects and dependenciesTree without any success.

In the meantime, I found the issue with our repository. At first, we wanted to use the same package versions as we did before the migration, but I noticed that pnpm sets link-workspace-packages to true by default. I tried to set it to false inside the subproject's folder (.npmrc), but for some reason, pnpm didn't take the settings into account. Because of this flag, pnpm tries to use the latest packages in the monorepo, causing this weird problem.

The list of relevant dependencies (and the respective versions):

Package Version
@nhost/core 0.9.1 (✅ Latest)
@nhost/nhost-js 1.5.2 (✅ Latest)
@nhost/nextjs 1.7.1 (❌ Latest is 1.8.1)
@nhost/react 0.12.1 (❌ Latest is 0.14.1)
@nhost/react-apollo 4.7.1 (❌ Latest is 4.8.1)

Note: The project has fixed dependency versions.

The error disappears as soon as I upgrade every dependency to the latest version, but I want to elaborate on the things I found, because this might be a problem somewhere else, not here.

For example here @nhost/react@0.12.1 depends on @nhost/nhost-js@1.4.8 (an earlier version) and @nhost/core@0.7.6 (also an earlier version). I was thinking this somehow can cause some weird state in the peer dependencies' cache for some reason, although I couldn't find out why.

What I found in the resolvePeers file is this: In the parentPkgs object, the node identifier of @nhost/core became link:packages/core, but this value is not available in the dependency tree. Every other node identifier is a dependency path (e.g: >dashboard/@nhost/nhost-js@1.4.8...) instead of a link, even @nhost/nhost-js, even though that's also using the latest version.

Maybe there is an obvious reason why this is happening, but I've reached a barrier here. Any help would be appreciated 🙏🏼

@zkochan
Copy link
Member

zkochan commented Nov 13, 2022

I tried to set it to false inside the subproject's folder (.npmrc)

it will probably only work in the .npmrc that is in the root of the workspace, where pnpm-workspace.yaml is.

@szilarddoro
Copy link
Author

I tried to set it to false inside the subproject's folder (.npmrc)

it will probably only work in the .npmrc that is in the root of the workspace, where pnpm-workspace.yaml is.

Yes, it worked in the root folder, but we wanted to set it on a per-project basis. Also, the per-project configuration is mentioned in the docs, so I thought it will work, but apparently, it didn't.

@zkochan
Copy link
Member

zkochan commented Nov 13, 2022

I don't understand why you want to set it per project. You can set it in the root and then use workspace:* in the dependencies to link projects from the workspace.

@szilarddoro
Copy link
Author

szilarddoro commented Nov 13, 2022

I don't understand why you want to set it per project. You can set it in the root and then use workspace:* in the dependencies to link projects from the workspace.

Because we want our example projects to always be copiable from the monorepo and to let people use them without any modifications. Therefore we can't set the dependencies to workspace:*, and therefore we can't disable this setting on a project level, because we want to keep dependency versions * in examples. We might rethink this in the future, but for now it is what it is.

Also, this configuration issue is not relevant to the problem I'm trying to solve in this PR.

@zkochan
Copy link
Member

zkochan commented Nov 13, 2022

The correct fix is in this PR: #5628

@zkochan zkochan closed this Nov 13, 2022
@zkochan zkochan added this to the v7.16 milestone Nov 14, 2022
@szilarddoro szilarddoro deleted the fix-resolve-peers branch November 14, 2022 07:35
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

Successfully merging this pull request may close these issues.

None yet

2 participants