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: search page for non latest doc versions #205

Merged
merged 5 commits into from
Jun 21, 2022

Conversation

Cyriuz
Copy link
Contributor

@Cyriuz Cyriuz commented May 30, 2022

fixes #203

Sorry for taking a while with this. I figured out the way to use the actual selected version as the useActiveVersion only works on doc pages (internally it just compares the path). However, this causes the test webpage to crash... so I spent yesterday trying to figure out why. After asking for help on the docusaurus discord it turns out that it is somehow because of the pnp build step:

[11:49] Josh-Cena: I have a feeling this has something to do with PnP
[11:49] Josh-Cena: Could you try with node-modules linker?
[11:50] Josh-Cena: I can't remember who was the last to bring it up, but there's a problem with PnP + contexts
[11:52] Josh-Cena: More specifically, theme-common fails to be deduplicated, so the context provider of theme-classic and the context consumer in the search theme are actually two instances, so they fail to pair up

After this I tried it in my own non pnp setup and it did indeed work. I tried changing some things to see if I could get it to work but I feel I'm a bit out of my depth here and may need some help. package dependencies seems like a pretty complicated field in its own...

@netlify
Copy link

netlify bot commented May 30, 2022

Deploy Preview for easyops-cn-docusaurus-search-local ready!

Name Link
🔨 Latest commit f29ec2d
🔍 Latest deploy log https://app.netlify.com/sites/easyops-cn-docusaurus-search-local/deploys/62b149c82d1d120009a4d2a0
😎 Deploy Preview https://deploy-preview-205--easyops-cn-docusaurus-search-local.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@weareoutman
Copy link
Member

@Cyriuz No sorry is needed at all, you helped a lot 👍. I'll look into it later, too, once I got time. If we couldn't find a solution with P'n'P, it's ok to use node-modules linker instead. But before that, give it some time for investigation.

@weareoutman
Copy link
Member

weareoutman commented May 31, 2022

I figured out that if we put the @docusaurus/theme-common into the peerDependencies of our theme (also add it to the devDependencies), will make it work with P'n'P. At the same time, we also need to explicitly add @docusaurus/theme-common into the dependencies of the example website.

I am wondering if it's the correct way, since our users may must manually add @docusaurus/theme-common into their website dependencies as we did here.

@weareoutman
Copy link
Member

I figured out that if we put the @docusaurus/theme-common into the peerDependencies of our theme (also add it to the devDependencies), will make it work with P'n'P. At the same time, we also need to explicitly add @docusaurus/theme-common to the dependencies of the example website.

I am wondering if it's the correct way, since our users may must manually add @docusaurus/theme-common to their website dependencies as we did here.

Maybe using peer dependencies is just the correct way, since our users may run into the same issue of duplicate instances of theme-common, if they didn't dedupe correctly.

In npm 7+ (shipped with node 15), npm will install peer dependencies automatically, so I think there will be no troubles these users will encounter. For others (npm <=6, yarn), I will first try to release an alpha version to test if it does work. If it requires an explicit dependencies of theme-common, we should add notice about it in installation section of our docs.

@Cyriuz
Copy link
Contributor Author

Cyriuz commented May 31, 2022

Awesome, yeah that sounds good! :D

@weareoutman
Copy link
Member

weareoutman commented Jun 13, 2022

This may take a little longer, I got very limit time on this project these days, :(

const activeVersion = useActiveVersion();
if (activeVersion && !activeVersion.isLast) {
baseUrl = activeVersion.path + "/";
const { pluginId } = useActivePlugin({failfast: true})!;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building the example website will fail due to this change.

➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/404" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/archive" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/hello-world" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/hola" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/tags" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/tags/docusaurus" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/tags/facebook" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/tags/hello" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/tags/hola" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/welcome" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/blog/writing-good-unit-tests" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/search" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs
➤ YN0000: [website]: Error: Can't find active docs plugin for "/docusaurus-search-local/" pathname, while it was expected to be found. Maybe you tried to use a docs feature that can only be used on a docs-related page? Existing docs plugin paths are: /docusaurus-search-local/docs

Copy link
Member

@weareoutman weareoutman Jun 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed that. However, I think we didn't fix #211

@weareoutman weareoutman marked this pull request as ready for review June 21, 2022 04:34
@weareoutman
Copy link
Member

weareoutman commented Jun 21, 2022

Tests passed, I think we're ready to go

@weareoutman weareoutman merged commit 91d82b1 into easyops-cn:master Jun 21, 2022
@slorber
Copy link

slorber commented Jul 6, 2022

Not sure why yet but it looks like this code makes Docusaurus fail on latest canary (but works on latest beta.21).

facebook/docusaurus#7686 (comment)

Currently investigating


Edit: finally it's not a problem in this package, see also: facebook/docusaurus#7728

@@ -10,7 +10,8 @@ import useDocusaurusContext from "@docusaurus/useDocusaurusContext";
import ExecutionEnvironment from "@docusaurus/ExecutionEnvironment";
import { useHistory, useLocation } from "@docusaurus/router";
import { translate } from "@docusaurus/Translate";
import { useActiveVersion } from '@docusaurus/plugin-content-docs/client';
import { useDocsPreferredVersion } from "@docusaurus/theme-common";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have moved this export to an internal entrypoint

import {
  useDocsPreferredVersion,
} from '@docusaurus/theme-common/internal';

This also means that technically we are not 100% ready to consider this API to be part of our public API surface and make no breaking changes on it 😅

But this can be discussed if you don't have a better choice than using it, and in practice we may not break it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any other way to do it so it is either that or if you know of any other, supported, way of getting the selected version?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will move useDocsPreferredVersion to our public api

Even though we may do a breaking change later, it will be clearly highlighted in a major release blog post

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Search page is not working with versioned docs
3 participants