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

theme-common doesn't deduplicate if site using pnpm/PnP installs it #7880

Open
7 tasks done
ndom91 opened this issue Aug 1, 2022 · 14 comments
Open
7 tasks done

theme-common doesn't deduplicate if site using pnpm/PnP installs it #7880

ndom91 opened this issue Aug 1, 2022 · 14 comments
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@ndom91
Copy link
Contributor

ndom91 commented Aug 1, 2022

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

So I've tried to copy the code from your own docs and make a custom category index page. With the sub-pages displayed as cards, and some additional text on the page. Therefore, I don't want to use the type: generate-index category page.

The pre-existing code worked fine in docusaurus v2.0.0-beta.21, however, when upgrading to v2.0.1 the page crashes upon visiting it in local dev, and Vercel preview deployments fail to build.

I can't reproduce the issue in a fresh Stackblitz actually. It works as expected there. However, here is my draft PR in which it does not work as expected anymore. As previously mentioned, the same code worked in v2.0.0-beta.21 (i.e. in our current main / prod deployment - https://next-auth.js.org/guides).

So when rendering the markdown below, I get the following error: Hook useDocsSidebar is called outside the <DocsSidebarProvider>. all of a sudden.

docs/guides/index.md

---
id: guides
title: Guides
---

# Guides

` ``mdx-code-block
import DocCardList from '@theme/DocCardList';
import {useCurrentSidebarCategory} from '@docusaurus/theme-common';

<DocCardList items={useCurrentSidebarCategory().items}/>
` ``

We have internal guides in three levels of difficulty.

If you can't find what you're looking for here, maybe take a look at our third-party [tutorials](/tutorials) page.

Reproducible demo

https://stackblitz.com/edit/github-e83uag?file=sidebars.js,package.json,docusaurus.config.js

Steps to reproduce

  1. Visit /guides page
  2. Watch <DocCardList /> render throw an error

Expected behavior

Render a custom category index page with the sub-pages listed in DocCardList components next to some more additional custom text.

Actual behavior

Opening the page in question causes the following error: Hook useDocsSidebar is called outside the <DocsSidebarProvider>.

image

Your environment

  • Public source code: https://github.com/nextauthjs/next-auth
  • Public site URL: https://next-auth.js.org
  • Docusaurus version used: 2.0.1
  • Environment name and version (e.g. Chrome 89, Node.js 16.4): 1.39.122 Chromium: 102.0.5005.115 (Official Build) (64-bit), Node 16.15.0
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): Arch Linux (up-to-date)

Self-service

  • I'd be willing to fix this bug myself.
@ndom91 ndom91 added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Aug 1, 2022
@ndom91 ndom91 changed the title <DocCardList /> not rendering in custom category index page situation <DocCardList /> crashing in custom category index page Aug 2, 2022
@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Aug 2, 2022

It is a known problem that using pnpm and other no-hoisting linking strategies (a.k.a. PnP) has caused theme-common to be duplicated in the package tree. In this case, the provider and the consumer of the same React context would be supplied by different copies of theme-common and cause this problem. (Also for reference: easyops-cn/docusaurus-search-local#205; I've answered this multiple times but I can't find other occurrences now)

docs$ pnpm why @docusaurus/theme-common
Legend: production dependency, optional only, dev only

next-auth-docs@0.2.0

dependencies:
@docusaurus/preset-classic 2.0.1
├─┬ @docusaurus/theme-classic 2.0.1
│ └── @docusaurus/theme-common 2.0.1 # This is the copy that theme-classic uses to render <DocsSidebarProvider>
├── @docusaurus/theme-common 2.0.1
└─┬ @docusaurus/theme-search-algolia 2.0.1
  └── @docusaurus/theme-common 2.0.1
@docusaurus/theme-common 2.0.1 # This is the copy that you use to call useCurrentSidebarCategory()

If you look into node_modules/.pnpm, you will see two copies of @docusaurus+theme-common as well, with different hashes.

It's not something I'd like to recommend, but I'd like to point out that adding public-hoist-pattern[]=@docusaurus/theme-common* to your .npmrc will make it work.

Again, I was pretty sure this had been brought up in the issue tracker but I can't find an open issue now, so I'll keep this open. We'll likely migrate theme-common to peer dependency some time in the future.

@Josh-Cena Josh-Cena changed the title <DocCardList /> crashing in custom category index page theme-common doesn't deduplicate if site installs it Aug 2, 2022
@Josh-Cena Josh-Cena changed the title theme-common doesn't deduplicate if site installs it theme-common doesn't deduplicate if site using pnpm/PnP installs it Aug 2, 2022
@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Aug 2, 2022
@ndom91
Copy link
Contributor Author

ndom91 commented Aug 2, 2022

@Josh-Cena okay great, thanks for the detailed answer! That worked. I did not find anything while searching, but was probably looking way too specifically for my issue, related to the sidebar items / DocsCardLink

The .npmrc change, however, did'nt have the desired effect. I had to manually delete the second @docusaurus/theme-common directory out of the node_modules/.pnpm dir for it to start working.

Any other idea how to do this programmatically so that automated CI builds on Vercel, for example, also work? @Josh-Cena

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Aug 5, 2022

Ah, forgot to mention—you need to remove it from your site dependencies as well. That's the point of using public-hoist-pattern😄

@slorber
Copy link
Collaborator

slorber commented Aug 9, 2022

It is a known problem that using pnpm and other no-hoisting linking strategies (a.k.a. PnP) has caused theme-common to be duplicated in the package tree. In this case, the provider and the consumer of the same React context would be supplied by different copies of theme-common and cause this problem.

Curious, why isn't this a problem for the ReactJS package in the first place? Just because we use peerDependencies for it?

@ndom91
Copy link
Contributor Author

ndom91 commented Aug 9, 2022

Ah, forgot to mention—you need to remove it from your site dependencies as well. That's the point of using public-hoist-pattern😄

Okay this ended up working for the @docusaudus/theme-common issue now 🎉

For some reason the marquee icon thing we use on our home page (https://next-auth.js.org - hero background) broke though with docusaurus v2 now. Throwing illegal hook call (useState) on render.

@Josh-Cena
Copy link
Collaborator

Curious, why isn't this a problem for the ReactJS package in the first place?

Yes. Everything is declaring react as a peer dependency, as they should do.

@slorber
Copy link
Collaborator

slorber commented Aug 9, 2022

For some reason the marquee icon thing we use on our home page (next-auth.js.org - hero background) broke though with docusaurus v2 now. Throwing illegal hook call (useState) on render.

So maybe it's a similar issue and React is duplicated by pnpm? 🤷‍♂️

Unrelated but that looks heavy to load styled-components just for that hero background slider 😅 (+ Docusaurus does not even have a proper SSR integration with that lib yet)

@ndom91
Copy link
Contributor Author

ndom91 commented Aug 9, 2022

For some reason the marquee icon thing we use on our home page (next-auth.js.org - hero background) broke though with docusaurus v2 now. Throwing illegal hook call (useState) on render.

So maybe it's a similar issue and React is duplicated by pnpm? man_shrugging

Unrelated but that looks heavy to load styled-components just for that hero background slider sweat_smile (+ Docusaurus does not even have a proper SSR integration with that lib yet)

Yes you're very right 😅 broke my heart, but we really love the animation 🎉

EDIT: Doesn't look like react is being duplicated. I double checked the pkg node_modules as well as the root node_modules 🤔. Also not sure what else could lead to this, but i'll keep digging.

@mikeechen
Copy link

@Josh-Cena is there still plan to move @docusaurus/theme-classic to peer dependency instead of keeping it in dependency? I've tried numerous approaches mentioned by #6724 and here, but nothing really prevails and I still get an error that #6724 mentioned. I do see two theme-common installed under two different hashes, but listing the packages to be hoisted didn't seem to have done anything on my side. I am using @microsoft/rushstack and pnpm under the hood though, but from what I'm seeing, rush just runs pnpm for install and doesn't interfere much with pnpm. I'm going to try a couple more times on the solutions provided, but it seems like listing them as peer dependency might solve most people's problems.

@mikeechen
Copy link

I ended up using pnpm feature to overwrite how dependency and peerDependency works in these packages documented here: https://pnpm.io/package_json#pnpmpackageextensions.

My configs ended up being:

	"packageExtensions": {
		"@docusaurus/theme-live-codeblock": {
			"peerDependencies": {
				"@docusaurus/theme-common": "*"
			},
			"dependencies": {
				"@docusaurus/theme-common": "*"
			}
		},
		"@docusaurus/preset-classic": {
			"peerDependencies": {
				"@docusaurus/theme-common": "*"
			},
			"dependencies": {
				"@docusaurus/theme-common": "*"
			}
		},
		"@docusaurus/theme-classic": {
			"peerDependencies": {
				"@docusaurus/theme-common": "*"
			},
			"dependencies": {
				"@docusaurus/theme-common": "*"
			}
		},
		"@docusaurus/theme-search-algolia": {
			"peerDependencies": {
				"@docusaurus/theme-common": "*"
			},
			"dependencies": {
				"@docusaurus/theme-common": "*"
			}
		}
	}

that seems to do the trick of making sure they all look at the same version of theme-common that I specifically added to the dependency.

@xllpiupiu
Copy link

It is a known problem that using pnpm and other no-hoisting linking strategies (a.k.a. PnP) has caused theme-common to be duplicated in the package tree. In this case, the provider and the consumer of the same React context would be supplied by different copies of theme-common and cause this problem. (Also for reference: easyops-cn/docusaurus-search-local#205; I've answered this multiple times but I can't find other occurrences now)

docs$ pnpm why @docusaurus/theme-common
Legend: production dependency, optional only, dev only

next-auth-docs@0.2.0

dependencies:
@docusaurus/preset-classic 2.0.1
├─┬ @docusaurus/theme-classic 2.0.1
│ └── @docusaurus/theme-common 2.0.1 # This is the copy that theme-classic uses to render <DocsSidebarProvider>
├── @docusaurus/theme-common 2.0.1
└─┬ @docusaurus/theme-search-algolia 2.0.1
  └── @docusaurus/theme-common 2.0.1
@docusaurus/theme-common 2.0.1 # This is the copy that you use to call useCurrentSidebarCategory()

If you look into node_modules/.pnpm, you will see two copies of @docusaurus+theme-common as well, with different hashes.

It's not something I'd like to recommend, but I'd like to point out that adding public-hoist-pattern[]=@docusaurus/theme-common* to your .npmrc will make it work.

Again, I was pretty sure this had been brought up in the issue tracker but I can't find an open issue now, so I'll keep this open. We'll likely migrate theme-common to peer dependency some time in the future.

it will not take effect when in monorep based on rush

@strmer15
Copy link

In my case, I actually had to add @docusaurus/types to the dependencies of my doc project - @docusaurus/theme-common was being duplicated by PNPM because @docusaurus/utils sometimes had the @docusaurus/types optional dependency provided and sometimes not. By providing it explicitly in my dependencies, it eliminated the duplicates and fixed my problem.

@HazyFish
Copy link

In my case, I actually had to add @docusaurus/types to the dependencies of my doc project - @docusaurus/theme-common was being duplicated by PNPM because @docusaurus/utils sometimes had the @docusaurus/types optional dependency provided and sometimes not. By providing it explicitly in my dependencies, it eliminated the duplicates and fixed my problem.

Thanks! It works for me!

@arobbins
Copy link

In my case, I actually had to add @docusaurus/types to the dependencies of my doc project - @docusaurus/theme-common was being duplicated by PNPM because @docusaurus/utils sometimes had the @docusaurus/types optional dependency provided and sometimes not. By providing it explicitly in my dependencies, it eliminated the duplicates and fixed my problem.

This fixed my Error: Cannot mix different versions of joi schemas issue after updating to v3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

No branches or pull requests

8 participants