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(v2): DocSearch should keep working after a new release (part 1/2) #3393

Merged
merged 4 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ Object {
"version-current-metadata-prop-751.json": "{
\\"version\\": \\"current\\",
\\"label\\": \\"Next\\",
\\"isLast\\": true,
\\"docsSidebars\\": {
\\"docs\\": [
{
Expand Down Expand Up @@ -615,6 +616,7 @@ Object {
"version-1-0-0-metadata-prop-608.json": "{
\\"version\\": \\"1.0.0\\",
\\"label\\": \\"1.0.0\\",
\\"isLast\\": true,
\\"docsSidebars\\": {
\\"version-1.0.0/community\\": [
{
Expand All @@ -631,6 +633,7 @@ Object {
"version-current-metadata-prop-751.json": "{
\\"version\\": \\"current\\",
\\"label\\": \\"Next\\",
\\"isLast\\": false,
\\"docsSidebars\\": {
\\"community\\": [
{
Expand Down Expand Up @@ -1073,6 +1076,7 @@ Object {
"version-1-0-0-metadata-prop-608.json": "{
\\"version\\": \\"1.0.0\\",
\\"label\\": \\"1.0.0\\",
\\"isLast\\": false,
\\"docsSidebars\\": {
\\"version-1.0.0/docs\\": [
{
Expand Down Expand Up @@ -1115,6 +1119,7 @@ Object {
"version-1-0-1-metadata-prop-e87.json": "{
\\"version\\": \\"1.0.1\\",
\\"label\\": \\"1.0.1\\",
\\"isLast\\": true,
\\"docsSidebars\\": {
\\"version-1.0.1/docs\\": [
{
Expand Down Expand Up @@ -1151,6 +1156,7 @@ Object {
"version-current-metadata-prop-751.json": "{
\\"version\\": \\"current\\",
\\"label\\": \\"Next\\",
\\"isLast\\": false,
\\"docsSidebars\\": {
\\"docs\\": [
{
Expand Down Expand Up @@ -1187,6 +1193,7 @@ Object {
"version-with-slugs-metadata-prop-2bf.json": "{
\\"version\\": \\"withSlugs\\",
\\"label\\": \\"withSlugs\\",
\\"isLast\\": false,
\\"docsSidebars\\": {
\\"version-1.0.1/docs\\": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ declare module '@docusaurus/plugin-content-docs-types' {
export type PropVersionMetadata = {
version: string;
label: string;
isLast: boolean;
docsSidebars: PropSidebars;
permalinkToSidebar: PermalinkToSidebar;
};
Expand Down
1 change: 1 addition & 0 deletions packages/docusaurus-plugin-content-docs/src/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export function toVersionMetadataProp(
return {
version: loadedVersion.versionName,
label: loadedVersion.versionLabel,
isLast: loadedVersion.isLast,
docsSidebars: toSidebarsProp(loadedVersion),
permalinkToSidebar: loadedVersion.permalinkToSidebar,
};
Expand Down
66 changes: 47 additions & 19 deletions packages/docusaurus-theme-classic/src/theme/DocPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import NotFound from '@theme/NotFound';
import type {DocumentRoute} from '@theme/DocItem';
import type {Props} from '@theme/DocPage';
import {matchPath} from '@docusaurus/router';
import Head from '@docusaurus/Head';

import styles from './styles.module.css';

Expand All @@ -27,34 +28,61 @@ type DocPageContentProps = {
readonly children: ReactNode;
};

// This theme is not coupled to Algolia, but can we do something else?
// Note the last version is also indexed with "last", to avoid breaking search on new releases
// See https://github.com/facebook/docusaurus/issues/3391
function DocSearchVersionHeader({
version,
isLast,
}: {
version: string;
isLast: boolean;
}) {
const versions = isLast ? [version, 'last'] : version;
return (
<Head>
<meta
name="docsearch:version"
content={
// See https://github.com/facebook/docusaurus/issues/3391#issuecomment-685594160
versions instanceof Array ? JSON.stringify(versions) : version
}
/>
</Head>
);
}

function DocPageContent({
currentDocRoute,
versionMetadata,
children,
}: DocPageContentProps): JSX.Element {
const {siteConfig, isClient} = useDocusaurusContext();
const {permalinkToSidebar, docsSidebars, version} = versionMetadata;
const {permalinkToSidebar, docsSidebars, version, isLast} = versionMetadata;
const sidebarName = permalinkToSidebar[currentDocRoute.path];
const sidebar = docsSidebars[sidebarName];
return (
<Layout version={version} key={isClient}>
<div className={styles.docPage}>
{sidebar && (
<div className={styles.docSidebarContainer} role="complementary">
<DocSidebar
sidebar={sidebar}
path={currentDocRoute.path}
sidebarCollapsible={
siteConfig.themeConfig?.sidebarCollapsible ?? true
}
/>
</div>
)}
<main className={styles.docMainContainer}>
<MDXProvider components={MDXComponents}>{children}</MDXProvider>
</main>
</div>
</Layout>
<>
<DocSearchVersionHeader version={version} isLast={isLast} />
<Layout key={isClient}>
<div className={styles.docPage}>
{sidebar && (
<div className={styles.docSidebarContainer} role="complementary">
<DocSidebar
sidebar={sidebar}
path={currentDocRoute.path}
sidebarCollapsible={
siteConfig.themeConfig?.sidebarCollapsible ?? true
}
/>
</div>
)}
<main className={styles.docMainContainer}>
<MDXProvider components={MDXComponents}>{children}</MDXProvider>
</main>
</div>
</Layout>
</>
);
}

Expand Down
2 changes: 0 additions & 2 deletions packages/docusaurus-theme-classic/src/theme/Layout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ function Layout(props: Props): JSX.Element {
image,
keywords,
permalink,
version,
} = props;
const metaTitle = title ? `${title} | ${siteTitle}` : siteTitle;
const metaImage = image || defaultImage;
Expand All @@ -63,7 +62,6 @@ function Layout(props: Props): JSX.Element {
{description && (
<meta property="og:description" content={description} />
)}
{version && <meta name="docsearch:version" content={version} />}
{keywords && keywords.length && (
<meta name="keywords" content={keywords.join(',')} />
)}
Expand Down
1 change: 0 additions & 1 deletion packages/docusaurus-theme-classic/src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ declare module '@theme/Layout' {
image?: string;
keywords?: string[];
permalink?: string;
version?: string;
};

const Layout: (props: Props) => JSX.Element;
Expand Down
2 changes: 1 addition & 1 deletion website/docusaurus.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ module.exports = {
apiKey: '47ecd3b21be71c5822571b9f59e52544',
indexName: 'docusaurus-2',
searchParameters: {
facetFilters: [`version:${versions[0]}`],
facetFilters: [`version:last`],
Copy link

@s-pace s-pace Sep 2, 2020

Choose a reason for hiding this comment

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

This change will make the search filters only on one version. The "last" one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is what we want no?

We already filtered on versions[0] anyway

So it means we had facetFilters: ["version:2.0.0-alpha.62"], and now we'll have this facetFilters: ["version:last"],

Copy link

Choose a reason for hiding this comment

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

Actually we would like the searchParameters to be injected from the version of the page you search on.

The current behaviour is not what we would expect. For example, searching on the page 2.0.0-alpha.58 filters on the latest version (alpha 62) while we would like to filter on the alpha 58.

Would it be possible to filter on the version of the page instead of only the latest?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@s-pace this is something we discussed with @francoischalifour already but we'll do that later.

Historically we only search in the last version. This PR only focus on not breaking search on new publish, due to the facetFilter changing name.

"contextual search" is more complicated to handle. We now support multiple docs plugin instances on same site, (like ios sdk 1.0/2.0 while android sdk is 1.0/1.1/1.2 etc...) so we should figure out a contextual search experience that account for this. Will be done in another PR later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So that we keep track of this, opened an issue: #3396

Copy link
Contributor

Choose a reason for hiding this comment

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

As a temporary solution you could also try the optionalFilters search parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, don't know much about Algolia apis :)

But this affects scoring yet would still allow to display duplicate results no? For example if a query match only 1 doc of each version, you'll still end up with results showing the same doc in multiple versions? I mean it just "boost" the score of one specific version right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm yes sorry, I just checked and it's not supported on the DocSearch plan anyway.

},
},
navbar: {
Expand Down