facebook / docusaurus

Easy to maintain open source documentation websites.
https://docusaurus.io
MIT License
54.9k stars 8.23k forks source link

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

Open ndom91 opened 2 years ago

ndom91 commented 2 years ago

Have you read the Contributing Guidelines on issues?

Prerequisites

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

Self-service

Josh-Cena commented 2 years ago

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: https://github.com/easyops-cn/docusaurus-search-local/pull/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.

ndom91 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 :tada:

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 :sweat_smile: broke my heart, but we really love the animation :tada:

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

mikeechen commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 9 months ago

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.

dayhaysoos commented 1 month ago

Is there a solution for this? I'm having this issue after I swizzled a component

Hook NavbarSecondaryMenuFiller is called outside the <NavbarSecondaryMenuContentProvider>.