-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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(theme-common): restore useContextualSearchFilters public API for retrocompatibility #10397
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: +407 B (0%) Total Size: 11.6 MB ℹ️ View Unchanged
|
const {i18n} = useDocusaurusContext(); | ||
const docsTags = | ||
// eslint-disable-next-line @typescript-eslint/no-var-requires | ||
require('@docusaurus/plugin-content-docs/client').useDocsContextualSearchTags(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this line to be the one causing me troubles on the official build of fast-check's documentation. This require makes theme-common
pull things from @docusaurus/plugin-content-docs
, which is just a peer.
The issue I'm facing appeared on 3.5.0. I need to come up with a repro implying a simplified version of my setup: yarn berry with docs being on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the issue I'm facing is probably that the very same 3.5.0 moved @docusaurus/plugin-content-docs
from dependency to peer 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand @dubzzz are you even using the fix in 3.5.2? Because you say you use 3.5.0
I can't help if I don't have a repro, or at least an error message or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My CI passes against 3.4.0 and fails with 3.5.2. I have not checked the intermediate versions yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would have been helpful to share the PR and CI job logs 😅
dubzzz/fast-check#5196
https://github.com/dubzzz/fast-check/actions/runs/10911561615/job/30284617054?pr=5196
Not sure why I can't see any CI logs there:
What do the logs say and how do you know it's related to this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it looks like, try using node linker instead
but we have e2e tests for Yarn / PnP so technically it should work, maybe it's a bug in a specific version of Yarn?
Our init template can build with Yarn 4.5.1 with PnP:
https://github.com/facebook/docusaurus/actions/runs/11611426378/job/32332737573
Using package extensions is a workaround that usually works to please PnP errors: https://yarnpkg.com/features/pnp#how-can-i-fix-ghost-dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the trick. I'll try it.
By the way I'm still trying to make up my mind on whether or not I stick to yarn or move to pnpm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange, I'm also using 4.5.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still like Yarn but this PnP thing is too innovative and always cause troubles 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It solved my issue. Thank you so much for the trick
Motivation
Restore theme-common API that we consider public.
This reverts an inadvertent breaking change I did while refactoring (#10316)
This breaking change breaks a local search plugin: cmfcmf/docusaurus-search-local#217
This will be released as v3.5.2
Test Plan
local
Test links
https://deploy-preview-10397--docusaurus-2.netlify.app/