refactor(docs,theme): split DocItem comp, useDoc hook by slorber · Pull Request #7644 · facebook/docusaurus

slorber · 2022-06-17T17:43:43Z

Breaking change

@theme/DocItem component has been split into much smaller subcomponents
@theme/DocItemFooter has been moved to @theme/DocItem/Footer and does not receive props anymore
You can access previous props data with the new useDoc() hook

Pre-flight checklist

I have read the Contributing Guidelines on pull requests.
If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

This is a follow-up of moving in the direction to:

using context instead of props => more flexible
split large component into smaller ones

We already did that in multiple places as well (doc sidebar etc...)

This PR does:

put doc route data in the context
access data through a new useDoc() hook, that will likely become part of our public API
define that public API more explicitly (ie we don't have to stick to the shape in which data is injected into comps => implementation detail)
split components into much smaller subcomponents, easier to swizzle
make it easy to change the doc item layout without swizzling a large file

We'll likely mark some of these components as safe to swizzle later?

Test Plan

Preview works as before

Test links

Deploy preview: https://deploy-preview-7644--docusaurus-2.netlify.app/docs

Related issues/PRs

Somehow related: #7637 (useDoc() will make this easy)

- put doc route data in context - access data through useDoc() hook - split components into much smaller subcomponents

netlify · 2022-06-17T17:49:14Z

✅ [V2]

Name	Link
🔨 Latest commit	`154a022`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/62b3022ca0822c0009067493
😎 Deploy Preview	https://deploy-preview-7644--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

github-actions · 2022-06-17T17:49:56Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🔴 28	🟢 100	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 84	🟢 100	🟢 100	🟢 100	🟢 90	Report

slorber

Waiting for a review before merge

slorber · 2022-06-17T17:44:56Z

packages/docusaurus-theme-classic/src/theme/DocItem/Layout/index.tsx

+/**
+ * Decide if the toc should be rendered, on mobile or desktop viewports
+ */
+function useDocTOC() {


maybe we'll want to extract this hook later?

Not 100% satisfied with it atm

I don't like this—seems like over-abstraction that hides its semantics.

What would you suggest?

I'd like the layout component to be easy to edit visually, without the TOC technical details noise.

Maybe it's fine to keep this weird hook, but just keep it there so that users can understand it more easily? 🤷‍♂️

I don't like it either but I don't have much better to suggest without polluting the component

Yes, when I'm swizzling, I'd personally like to gain some understanding of what the hook is doing. This is especially important for non-TS users. This hook is congregating too many unrelated things. It's fine if it's in the same file and easily viewable, but extracting it to theme-common makes it much harder to work with.

agree 👍

let's ship this as is and see how to improve later

packages/docusaurus-theme-common/src/contexts/doc.tsx

slorber · 2022-06-17T17:52:04Z

packages/docusaurus-theme-common/src/contexts/doc.tsx

+ * but we may want to change that in the future.
+ * This layer is a good opportunity to decouple storage from consuming API.
+ */
+function useContextValue(content: PropDocContent): DocContextValue {


My opinion: storing metadata/frontmatter in the mdx file is an implementation detail. Consuming the data with Component.metadata doesn't feel very natural to me so I'd rather not stick to it 🤷‍♂️

In practice, we could create separate bundles/props for these metadata. And it may have advantages to do so in the future if for example we want to provide some kind of "data query layer" (I mean, for example we want to display some docs/blog metadata in the homepage, without having to load all the mdx content in that page)

I'm not sure how things will turn out, but I'd prefer to decouple the metadata from the MDX component when consuming it. I'd like to do something similar on the blog as well.

I agree—the metadata should be a separate chunk. This means we can easily provide the same metadata as both props and route context. Seems too complex in the short term, though.

github-actions · 2022-06-17T17:57:09Z

Size Change: +18 B (0%)

Total Size: 801 kB

ℹ️ View Unchanged

Filename	Size	Change
`website/.docusaurus/globalData.json`	52.6 kB	0 B
`website/build/assets/css/styles.********.css`	106 kB	0 B
`website/build/assets/js/main.********.js`	603 kB	+18 B (0%)
`website/build/index.html`	38.9 kB	0 B

_{compressed-size-action}

packages/docusaurus-theme-classic/src/theme-classic.d.ts

packages/docusaurus-theme-common/src/contexts/doc.tsx

Josh-Cena · 2022-06-22T11:00:20Z

packages/docusaurus-theme-common/src/contexts/doc.tsx

+function useContextValue(content: PropDocContent): DocContextValue {
+  return useMemo(
+    () => ({
+      MDXComponent: content,


Do we want to expose this as context? Looks like TMI to me 😄

That's part of the static data being available in the subtree, so not sure how it could be used, but why not?

Eventually we may move all this subtree context data to be made available at the plugin level? Using our secret useRouteContext? #6885
(I mean useDoc() ➡️ useRouteContext().doc, no need for DocContext)

You'd prefer to forward a children prop?

I think both solutions are fine 🤷‍♂️ it's unlikely someone would want to render that component in a weird place like the Footer, so forwarding children could also work

I was thinking this is useless in all cases to be exposed as a context, since only the main container would care about it and it doesn't really encourage any code reuse. (e.g. no one would be forwarding this to some subcomponent as props.)

Up to you, I'm fine if you revert it to children if you think it's better, both look fine to me

Josh-Cena · 2022-06-22T12:02:43Z

packages/docusaurus-theme-common/src/contexts/doc.tsx

+  return useMemo(
+    () => ({
+      metadata: content.metadata,
+      frontMatter: content.frontMatter,
+      assets: content.assets,
+      contentTitle: content.contentTitle,
+      toc: content.toc,
+    }),


Are we intentionally cherry-picking the properties from content on both type and value, instead of using a spread?

yes because I'd like to avoid mdx loader export implementation details to automatically leak into an API that we'll likely make public later

Makes sense. Just confirming

lanegoolsby · 2022-08-04T17:49:47Z

@slorber is useDoc exported? I tried to import it in my component but get an error:

import React, { useState } from 'react';
import TOC from '@theme-original/TOC';
import { useDoc } from '@docusaurus/theme-common/src/contexts/doc';

export default function TOCWrapper(props: any) {
  const docMeta = useDoc();
  const [tags, setTags] = useState<any[]>([]);

  function asdf() {
    if (docMeta.frontMatter.tags && docMeta.frontMatter.tags.length > 0) {
      setTags(docMeta.frontMatter.tags);
    }
  }

  return (
    <>
      <TOC {...props} />
      Hello world
    </>
  );
}

Module not found: Error: Package path ./src/contexts/doc is not exported from package /my/repo/path/node_modules/@docusaurus/theme-common (see exports field in /my/repo/path/node_modules/@docusaurus/theme-common/package.json)

TasoOneAsia · 2022-08-04T20:21:56Z

@lanegoolsby Was looking into utilizing this hook as well, and from preliminary investigation seems like the useDoc hook is only being exported under the internal entry point for the time being. Should be able to import it by referencing that sub-path instead, i.e.

import { useDoc } from '@docusaurus/theme-common/internal'

lanegoolsby · 2022-08-05T01:01:21Z

Typescript and/or VSCode doesn't like that

Josh-Cena · 2022-08-05T01:34:33Z

@lanegoolsby You need to compile with moduleResolution: NodeNext.

lanegoolsby · 2022-08-08T16:52:07Z

Can this hook be used in a custom plugin? I can use it in a swizzled TOC, but I get this error when I try to use the same code in a custom plugin.

I am using a /docs path - /docs/tutorial-basics/create-a-document

I tried both with and without a <DocProvider>, both throw the same error.

import React, { useState } from 'react';
import { useDoc } from '@docusaurus/theme-common/internal';
import TOC from '@theme-init/TOC';
import { DocProvider } from '@docusaurus/theme-common/internal';

export default function (props: any) {
  const docMeta = useDoc();

  return (
    <DocProvider content={props.content}>
      <TOC {...props} />
      Hello world <code>{JSON.stringify(docMeta)}</code>
    </DocProvider>
  );
}

slorber · 2022-08-08T17:22:46Z

useDoc() is only supposed to be used on a doc page. Other pages (blog, pages or your custom plugin routes) do not have the same data available so you'd have to use another hook, eventually creating your own.

I'm not sure to understand at all what you are trying to do here 😅 please show a repro.

Note technically you could reuse DocProvider + useDoc (a React context is just a way to provide a value to a whole subtree), but these comp/hook are created with TS typings for the data made available through the docs plugin, so if you are looking to pass a very different set of data you'd rather create your own React context instead.

Note: you can't call the useXyz() hook in the same component that uses the provider <ProvideXyz>. What you do above is always wrong. You can only use the hook on components in the provider subtree. You can generally solve this problem by extracting what's inside the provider to a new custom React component, and using the hook there.

lanegoolsby · 2022-08-08T17:41:03Z

I'm trying to setup a custom plugin that injects a custom React component immediately below the Table of Contents component, for example:

Where the Hello World is, I'd like to have a component that will read all the tags for a doc page, query an internal API with said tags via HTTP/REST, and render the results in a grid.

slorber · 2022-08-09T08:12:55Z

I'm trying to setup a custom plugin that injects a custom React component immediately below the Table of Contents component

Note the TOC component is more a "design system component" and it's not really meant to be coupled to a specific usage (docs, blog, pages). I mean it's not supposed to use useDoc() or useBlog(). Now we don't prevent it but I think you'd rather avoid creating such coupling.

If you only want to enhance the Doc TOC (and not the page/blog TOC), you should rather swizzle
@theme/DocItem/TOC/Desktop (instead of @theme/TOC) => this component can safely use useDoc()

Also don't forget to think about the mobile experience where the TOC is a collapsible at the top of the document

Refactor DocItem:

40b2526

- put doc route data in context - access data through useDoc() hook - split components into much smaller subcomponents

slorber requested review from Josh-Cena and lex111 as code owners June 17, 2022 17:43

facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 17, 2022

slorber commented Jun 17, 2022

View reviewed changes

slorber added the pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. label Jun 17, 2022

Josh-Cena reviewed Jun 18, 2022

View reviewed changes

packages/docusaurus-theme-classic/src/theme-classic.d.ts Outdated Show resolved Hide resolved

Merge branch 'main' into slorber/current-doc-provider

2331941

Josh-Cena changed the title ~~refactor(docs,theme): refactor DocItem: split in small comps + useDoc()~~ refactor(docs,theme): split DocItem comp, useDoc hook Jun 22, 2022

@theme/DocItem/Footer

b93fb1f

Josh-Cena reviewed Jun 22, 2022

View reviewed changes

packages/docusaurus-theme-common/src/contexts/doc.tsx Show resolved Hide resolved

slorber and others added 2 commits June 22, 2022 12:47

better doc

b0b296a

Improve doc

f7f0fab

Josh-Cena reviewed Jun 22, 2022

View reviewed changes

This looks better to me

154a022

Josh-Cena reviewed Jun 22, 2022

View reviewed changes

Josh-Cena approved these changes Jun 22, 2022

View reviewed changes

slorber merged commit fd87afd into main Jun 22, 2022

slorber deleted the slorber/current-doc-provider branch June 22, 2022 12:27

This was referenced Jul 1, 2022

refactor(theme): split BlogPostItem into smaller theme subcomponents #7716

Merged

chore: prepare v2.0.0-beta.22 release #7741

Merged

dipakparmar mentioned this pull request Jul 10, 2022

⬆️ bump @docusaurus/* from 2.0.0-beta.21 to 2.0.0-beta.22 dipakparmar/docs-diary#55

Merged

blindaa121 mentioned this pull request Jul 11, 2022

Incompatible with Beta22 PaloAltoNetworks/docusaurus-openapi-docs#151

Closed

tats-u mentioned this pull request Feb 26, 2023

Docusaurusのバージョンを2.0.0-beta.14から2.3.1以上にアップデートしませんか？ yytypescript/book#600

Closed

Uh oh!

Conversation

slorber commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking change

Pre-flight checklist

Motivation

Test Plan

Test links

Related issues/PRs

Uh oh!

netlify bot commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ [V2]

Uh oh!

github-actions bot commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚡️ Lighthouse report for the deploy preview of this PR

Uh oh!

slorber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Josh-Cena Jun 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slorber Jun 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lanegoolsby commented Aug 4, 2022

Uh oh!

TasoOneAsia commented Aug 4, 2022

Uh oh!

lanegoolsby commented Aug 5, 2022

Uh oh!

Josh-Cena commented Aug 5, 2022

Uh oh!

lanegoolsby commented Aug 8, 2022

Uh oh!

slorber commented Aug 8, 2022

Uh oh!

lanegoolsby commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber commented Aug 9, 2022

Uh oh!

Reviewers

Assignees

Labels

slorber commented Jun 17, 2022 •

edited

Loading

netlify bot commented Jun 17, 2022 •

edited

Loading

github-actions bot commented Jun 17, 2022 •

edited

Loading

Josh-Cena Jun 22, 2022 •

edited

Loading

github-actions bot commented Jun 17, 2022 •

edited

Loading

slorber Jun 22, 2022 •

edited

Loading

lanegoolsby commented Aug 8, 2022 •

edited

Loading