fix(v2): fix docs homepage permalink issues #2905

slorber · 2020-06-08T15:06:08Z

Related to issues:

Current docs homepage code almost works but introduce workarounds at the "edges" (for example we have to handle if/else logic specific to the homepage in the theme components).

I think we should do the opposite, fix the feature at the core directly in the metadata, so that it works once and for all.

I've been able to simplify the docs homepage code, as well as fixing some issues at the same time (like the "previous" link not pointing correctly to /docs).

The idea is simple:

We should add a single /docs route per version, with exact: false, so that it can match both /docs and /docs/*
Docs home should have permalink: /docs/

This way, we are able to handle the homepage like any regular page, with the same data file and the same render path (renderRoutes()).

Having a single parent route is also a bit better for perf, because it permits to avoid unmounting/remounting the docs layout when navigation from /docs/ to /docs/nextDoc.

This enabled me to remove some code introduced in the themes, in both classic/bootstrap.

Even if it renders correctly for /docs, I had to put a trailing / in the permalink (so the canonical docs home is /docs/ not /docs), not easy to fix and probably not a big deal.

docusaurus-bot · 2020-06-08T15:15:53Z

Deploy preview for docusaurus-2 ready!

Built with commit 258c6ac

https://deploy-preview-2905--docusaurus-2.netlify.app

slorber · 2020-06-08T16:26:09Z

Hi @lex111 @fanny , this PR is ready to review.

I was able to remove a few lines of code and simplify the docs home implementation. The themes should not need to handle the homepage in a specific way anymore.

This also fixes the "previous" link here: #2896 (comment)

slorber · 2020-06-08T16:35:25Z

packages/docusaurus-plugin-content-docs/src/index.ts

-
-    return false;
-  };
-


@lex111 code I was able to delete by using permalink = / in metadatas

slorber · 2020-06-08T16:35:35Z

packages/docusaurus-plugin-content-docs/src/index.ts

-                  content: metadataItem.source,
-                },
-              });
-            }


@lex111 code I was able to delete by using permalink = / in metadatas

slorber · 2020-06-08T16:40:29Z

packages/docusaurus-theme-bootstrap/src/theme/DocPage/index.js

-          ) : (
-            renderRoutes(baseRoute.routes)
-          )}
+          {renderRoutes(baseRoute.routes)}


@lex111 was able to revert the changes that are no longer required for docs homepage support

@fanny actually as it's bootstrap :)

slorber · 2020-06-08T16:41:12Z

packages/docusaurus-theme-classic/src/theme/DocPage/index.js

-            ) : (
-              renderRoutes(baseRoute.routes)
-            )}
+            {renderRoutes(baseRoute.routes)}


@lex111 was able to revert the changes that are no longer required for docs homepage support

slorber · 2020-06-08T16:41:48Z

packages/docusaurus-theme-classic/src/theme/DocSidebar/index.js

 const isActiveSidebarItem = (item, activePath) => {
  if (item.type === 'link') {
-    return item.href === activePath;
+    return isSamePath(item.href, activePath);


consider /docs is active if path is /docs/

slorber · 2020-06-08T16:44:13Z

packages/docusaurus-plugin-content-docs/src/metadata.ts

+    throw new Error(
+      `The docs homepage (homePageId=${homePageId}) is not allowed to have a frontmatter slug=${frontMatter.slug} => you have to chooser either homePageId or slug, not both`,
+    );
+  }


@lex111 introduce a new warning, as discussed in #2879

As we don't duplicate the docs home (ie no /docs + /docs/introduction), then we should rather prevent the user to use the :slug frontmatter, as that wouldn't make sense)

lex111

I feel so dumb because I really could go this easier, more natural way to implement the docs homepage feature 🤦

@slorber alright, thank you for carrying that forward to its meaningful conclusion!

However, I noticed some bugs:

"Nested" docs (with slashes/dirs) no longer work if you define it in homePageId (e.g. doc_cat/intro). You can look at an example on Chart.js https://github.com/chartjs/Chart.js/blob/master/docs/sidebars.js#L8
When in docs-only mode enabled (set to routeBasePath: '/') and homePageId set with any value (eg introduction), all docs routes display content for specified homePageId, and without a layout website:

slorber · 2020-06-09T12:53:23Z

I feel so dumb because I really could go this easier, more natural way to implement the docs homepage feature 🤦

That happens 🤪

Thanks, will take a look to fix that.

I'm not sure to understand what you mean by nested docs for now ^^

…ncluding /) should be used to compare against homePageId

…for all versions

slorber · 2020-06-09T18:22:47Z

@lex111

I fixed the first issue related to docs in subfolders, deployed in the preview a test:

https://deploy-preview-2905--docusaurus-2.netlify.app/docs/next/

BTW this raises another question that's visible in the: as I selected as home doc id a new doc, and older versions do not have this doc, this basically breaks the homepage of other versions.

The home of current version is now broken:

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

Shouldn't we version this homeDocId? cc @yangshun

It looks like I should be able to say:

current version home is introduction
next version home is getting-started

For the 2nd issue about / as root for docs, that's quite complicated 🤪
It goes deep into the routesChunkName.json file, as 2 routes (docItem/docPage) tries to write the entry for "/", and we end up with a bad merge:

  "/": {
    "component": "component---theme-doc-item-178-a40",
    "docsMetadata": "docsMetadata---ec-1-ddf",
    "content": "content---ec-6-8fc"
  },

Before we had:

  "/": {
    "component": "component---theme-doc-page-1-be-9be",
    "docsMetadata": "docsMetadata---c-78-ad7",
    "content": "content---ec-6-8fc"
  },
  "/:route": {
    "component": "component---theme-doc-page-1-be-9be",
    "docsMetadata": "docsMetadata---routeec-1-f92"
  },

Will try to find a solution, not so simple :)

edit: found a workaround => #2917

BTW the current master isn't working perfectly either:

incorrect sidebar highlighting
the homepage works, but the sidebar link to the homepage (installation) does not lead anywhere

yangshun · 2020-06-10T04:11:10Z

Shouldn't we version this homeDocId?

Yes I think so, but not sure how to do so or if it's even possible, given that we don't version the docusaurus.config.

slorber · 2020-06-10T10:51:20Z

Yes I think so, but not sure how to do so or if it's even possible, given that we don't version the docusaurus.config.

Same, but not sure how to do that without introducing a new versioned file 🤪 I think it can be fine for now, it probably does not happen often that the user decides to change the homepage. Likely, The homepage feature is likely the most important for the current version IMHO, it's probably not a big deal if the user does not have the homepage on /next if he does the change.

One hacky way to solve that is to allow passing an array: homePageId: ["newNextVersionHomePageId","oldHomePageId"]. I'd rather not add that but this is a potential hack we can add if users complain.

slorber · 2020-06-10T14:19:28Z

Hi @lex111

The first issue: already solved (currently deployed to netlify, will revert soon to prepare for merge)

The 2nd issue:
I've found a bug where the parent route ("/": docs layout/DocPage) is the same as the child route ("/": home of docs/DocItem).
Documented this here: #2917

I was able to find a workaround by using "" as parent route and "/" as child route, "/next" as parent route, and "/next/" as child route and it seems to work fine.

Here's a deployment if you want to test easily:

https://focused-hoover-167617.netlify.app/
https://focused-hoover-167617.netlify.app/next
https://focused-hoover-167617.netlify.app/2.0.0-alpha.50/

Do you see any problem?

…ath as the child route (DocItem): see facebook#2917

slorber · 2020-06-10T15:34:30Z

packages/docusaurus-plugin-content-docs/src/index.ts

+        // Important: the layout component should not end with /,
+        // as it conflicts with the home doc
+        // Workaround fix for https://github.com/facebook/docusaurus/issues/2917
+        const path = docsBaseRoute === '/' ? '' : docsBaseRoute;


@lex111 This is the workaround fix for the 2nd issue (ie base path for / is "", not "/", to not conflict with home doc which is also "/")

slorber · 2020-06-10T15:36:27Z

packages/docusaurus-plugin-content-docs/src/metadata.ts

+  const id = dirName !== '.' ? `${dirName}/${baseID}` : baseID;
+  const idWithoutVersion = version ? removeVersionPrefix(id, version) : id;
+
+  const isDocsHomePage = idWithoutVersion === homePageId;


@lex111 this is the fix for the 1st issue you reported

slorber · 2020-06-12T16:48:01Z

@lex111 @yangshun can you review this please?

# Conflicts: # packages/docusaurus-plugin-content-docs/src/index.ts

better fixes for docs homepage

6734005

slorber requested review from lex111 and yangshun as code owners June 8, 2020 15:06

facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 8, 2020

fix tests

bfe01e2

slorber changed the title ~~fix homepage permalink~~ bug(v2): fix docs homepage permalink issues Jun 8, 2020

slorber added 3 commits June 8, 2020 18:05

create special route for docs homepage + cleanup existing code

32790d8

no need to create multiple docs parent paths

edd5a1d

useful comment

280f08f

slorber commented Jun 8, 2020

View reviewed changes

slorber added 3 commits June 8, 2020 18:54

add test for slug + doc home usage at the same time error

8e91f2b

remove confusing variable name

3537cb8

fix tests by using same suffix as before for docs base metadata path

ba27841

lex111 reviewed Jun 8, 2020

View reviewed changes

slorber added 4 commits June 9, 2020 16:43

Merge branch 'master' into refactor/docs-home-url

179f27f

metadata: use homePageId correctly for nested docs: the full docId (i…

5e7ee40

…ncluding /) should be used to compare against homePageId

add folder/testNested test doc

e996ad2

refactor a bit processMetadata, the home should be handled correctly …

66cd083

…for all versions

slorber mentioned this pull request Jun 10, 2020

feat(v2): add a banner that links to latest version of documentation #2916

Merged

slorber mentioned this pull request Jun 10, 2020

routesChunkName.json does not allow sub-route with same path as parent. #2917

Closed

slorber added 4 commits June 10, 2020 16:21

Workaround to fix issue when parent layout route (DocPage) has same p…

fd94d28

…ath as the child route (DocItem): see facebook#2917

revert homePageId

268f964

remove test doc

0c0b6d1

remove test doc

74d84d9

slorber requested a review from lex111 June 10, 2020 15:30

slorber commented Jun 10, 2020

View reviewed changes

add useful comment

829186c

slorber commented Jun 10, 2020

View reviewed changes

slorber mentioned this pull request Jun 11, 2020

Using _index id in sidebar.js #2920

Closed

slorber linked an issue Jun 11, 2020 that may be closed by this pull request

Using _index id in sidebar.js #2920

Closed

Merge branch 'master' into refactor/docs-home-url

258c6ac

# Conflicts: # packages/docusaurus-plugin-content-docs/src/index.ts

yangshun approved these changes Jun 16, 2020

View reviewed changes

slorber changed the title ~~bug(v2): fix docs homepage permalink issues~~ fix(v2): fix docs homepage permalink issues Jun 17, 2020

slorber added pr: bug fix This PR fixes a bug in a past release. pr: maintenance This PR does not produce any behavior differences to end users when upgrading. and removed pr: maintenance This PR does not produce any behavior differences to end users when upgrading. labels Jun 17, 2020

slorber merged commit f6b1c85 into facebook:master Jun 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(v2): fix docs homepage permalink issues #2905

fix(v2): fix docs homepage permalink issues #2905

slorber commented Jun 8, 2020 •

edited

Loading

docusaurus-bot commented Jun 8, 2020 •

edited

Loading

slorber commented Jun 8, 2020

slorber Jun 8, 2020

slorber Jun 8, 2020

slorber Jun 8, 2020

slorber Jun 8, 2020

slorber Jun 8, 2020

slorber Jun 8, 2020

slorber Jun 8, 2020

lex111 left a comment

slorber commented Jun 9, 2020

slorber commented Jun 9, 2020 •

edited

Loading

yangshun commented Jun 10, 2020

slorber commented Jun 10, 2020

slorber commented Jun 10, 2020

slorber Jun 10, 2020

slorber Jun 10, 2020

slorber commented Jun 12, 2020

fix(v2): fix docs homepage permalink issues #2905

fix(v2): fix docs homepage permalink issues #2905

Conversation

slorber commented Jun 8, 2020 • edited Loading

docusaurus-bot commented Jun 8, 2020 • edited Loading

slorber commented Jun 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lex111 left a comment

Choose a reason for hiding this comment

slorber commented Jun 9, 2020

slorber commented Jun 9, 2020 • edited Loading

yangshun commented Jun 10, 2020

slorber commented Jun 10, 2020

slorber commented Jun 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slorber commented Jun 12, 2020

slorber commented Jun 8, 2020 •

edited

Loading

docusaurus-bot commented Jun 8, 2020 •

edited

Loading

slorber commented Jun 9, 2020 •

edited

Loading