From 868d72fe4f855f847cc258760a9b25b629492284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lorber?= Date: Fri, 19 Jul 2024 17:13:28 +0200 Subject: [PATCH] fix(core): revert wrong anchor link implementation change (#10311) --- .github/workflows/build-perf.yml | 10 +- .../docusaurus/src/client/exports/Link.tsx | 20 +- .../client/exports/__tests__/Link.test.tsx | 347 ++++++++++++++++++ 3 files changed, 371 insertions(+), 6 deletions(-) create mode 100644 packages/docusaurus/src/client/exports/__tests__/Link.test.tsx diff --git a/.github/workflows/build-perf.yml b/.github/workflows/build-perf.yml index 533a4ade0015..72061aad3be3 100644 --- a/.github/workflows/build-perf.yml +++ b/.github/workflows/build-perf.yml @@ -46,9 +46,11 @@ jobs: uses: preactjs/compressed-size-action@f780fd104362cfce9e118f9198df2ee37d12946c # v2 with: repo-token: ${{ secrets.GITHUB_TOKEN }} - build-script: build:website:en + build-script: build:website:fast clean-script: clear:website # see https://github.com/facebook/docusaurus/pull/6838 - pattern: '{website/build/assets/js/main*js,website/build/assets/css/styles*css,website/.docusaurus/globalData.json,website/.docusaurus/registry.js,website/.docusaurus/routes.js,website/.docusaurus/routesChunkNames.json,website/.docusaurus/site-metadata.json,website/.docusaurus/codeTranslations.json,website/.docusaurus/i18n.json,website/.docusaurus/docusaurus.config.mjs,website/build/index.html,website/build/blog/index.html,website/build/blog/**/introducing-docusaurus/*,website/build/docs/index.html,website/build/docs/installation/index.html,website/build/tests/docs/index.html,website/build/tests/docs/standalone/index.html}' + pattern: '{website/build/assets/js/main*js,website/build/assets/css/styles*css,website/.docusaurus/globalData.json,website/.docusaurus/registry.js,website/.docusaurus/routes.js,website/.docusaurus/routesChunkNames.json,website/.docusaurus/site-metadata.json,website/.docusaurus/codeTranslations.json,website/.docusaurus/i18n.json,website/.docusaurus/docusaurus.config.mjs,website/build/index.html,website/build/docs.html,website/build/docs/**/*.html,website/build/blog.html,website/build/blog/**/*.html}' + # HTML files: exclude versioned docs pages, tags pages, html redirect files + exclude: '{website/build/docs/?.?.?/**/*.html,website/build/docs/next/**/*.html,website/build/blog/tags/**/*.html,**/*.html.html}' strip-hash: '\.([^;]\w{7})\.' minimum-change-threshold: 30 compression: none @@ -68,11 +70,11 @@ jobs: # Ensure build with a cold cache does not increase too much - name: Build (cold cache) - run: yarn workspace website build --locale en + run: yarn build:website:fast timeout-minutes: 8 # Ensure build with a warm cache does not increase too much - name: Build (warm cache) - run: yarn workspace website build --locale en + run: yarn build:website:fast timeout-minutes: 2 # TODO post a GitHub comment with build with perf warnings? diff --git a/packages/docusaurus/src/client/exports/Link.tsx b/packages/docusaurus/src/client/exports/Link.tsx index 08901b9bd4c4..0f3cacb77614 100644 --- a/packages/docusaurus/src/client/exports/Link.tsx +++ b/packages/docusaurus/src/client/exports/Link.tsx @@ -135,7 +135,7 @@ function Link( useEffect(() => { // If IO is not supported. We prefetch by default (only once). - if (!IOSupported && isInternal) { + if (!IOSupported && isInternal && ExecutionEnvironment.canUseDOM) { if (targetLink != null) { window.docusaurus.prefetch(targetLink); } @@ -157,7 +157,15 @@ function Link( const hasInternalTarget = !props.target || props.target === '_self'; // Should we use a regular tag instead of React-Router Link component? - const isRegularHtmlLink = !targetLink || !isInternal || !hasInternalTarget; + const isRegularHtmlLink = + !targetLink || + !isInternal || + !hasInternalTarget || + // When using the hash router, we can't use the regular link for anchors + // We need to use React Router to navigate to /#/pathname/#anchor + // And not /#anchor + // See also https://github.com/facebook/docusaurus/pull/10311 + (isAnchorLink && router !== 'hash'); if (!noBrokenLinkCheck && (isAnchorLink || !isRegularHtmlLink)) { brokenLinks.collectLink(targetLink!); @@ -167,6 +175,12 @@ function Link( brokenLinks.collectAnchor(props.id); } + // These props are only added in unit tests to assert/capture the type of link + const testOnlyProps = + process.env.NODE_ENV === 'test' + ? {'data-test-link-type': isRegularHtmlLink ? 'regular' : 'react-router'} + : {}; + return isRegularHtmlLink ? ( // eslint-disable-next-line jsx-a11y/anchor-has-content, @docusaurus/no-html-links ) : ( ); } diff --git a/packages/docusaurus/src/client/exports/__tests__/Link.test.tsx b/packages/docusaurus/src/client/exports/__tests__/Link.test.tsx new file mode 100644 index 000000000000..392336a984d6 --- /dev/null +++ b/packages/docusaurus/src/client/exports/__tests__/Link.test.tsx @@ -0,0 +1,347 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +/* eslint-disable jsx-a11y/anchor-is-valid */ + +import React, {type ReactNode} from 'react'; +import renderer from 'react-test-renderer'; +import {fromPartial} from '@total-typescript/shoehorn'; +import {StaticRouter} from 'react-router-dom'; +import Link from '../Link'; +import {Context} from '../../docusaurusContext'; +import type {DocusaurusContext} from '@docusaurus/types'; + +type Options = { + trailingSlash: boolean | undefined; + baseUrl: string; + router: DocusaurusContext['siteConfig']['future']['experimental_router']; + currentLocation: string; +}; + +const defaultOptions: Options = { + trailingSlash: undefined, + baseUrl: '/', + router: 'browser', + // currentLocation is nested on purpose, shows relative link resolution + currentLocation: '/sub/category/currentPathname', +}; + +function createDocusaurusContext( + partialOptions: Partial, +): DocusaurusContext { + const options: Options = {...defaultOptions, ...partialOptions}; + return fromPartial({ + siteConfig: { + baseUrl: options.baseUrl, + trailingSlash: options.trailingSlash, + future: { + experimental_router: options.router, + }, + }, + }); +} + +function createLinkRenderer(defaultRendererOptions: Partial = {}) { + return (linkJsx: ReactNode, testOptions: Partial = {}) => { + const options: Options = { + ...defaultOptions, + ...defaultRendererOptions, + ...testOptions, + }; + const docusaurusContext = createDocusaurusContext(options); + return renderer + .create( + + + {linkJsx} + + , + ) + .toJSON(); + }; +} + +describe('', () => { + describe('using "browser" router', () => { + const render = createLinkRenderer({router: 'browser'}); + + it("can render '/docs/intro'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render '/docs/intro' with baseUrl /baseUrl/", () => { + expect(render(, {baseUrl: '/baseUrl/'})) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render '/docs/intro' with baseUrl /docs/", () => { + // TODO Docusaurus v4 ? + // Change weird historical baseUrl behavior + // we should link to /docs/docs/intro, not /docs/intro + // see https://github.com/facebook/docusaurus/issues/6294 + expect(render(, {baseUrl: '/docs/'})) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render '/docs/intro' with trailingSlash true", () => { + expect(render(, {trailingSlash: true})) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render '/docs/intro/' with trailingSlash false", () => { + expect(render(, {trailingSlash: false})) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render '#anchor'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render '/docs/intro#anchor'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render '/docs/intro/#anchor'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render '/pathname?qs#anchor'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render ''", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render 'relativeDoc'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render './relativeDoc'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render './../relativeDoc?qs#anchor'", () => { + expect(render()) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render 'https://example.com/xyz'", () => { + expect(render()) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render 'pathname:///docs/intro'", () => { + expect(render()) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render 'pathname://docs/intro'", () => { + expect(render()) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render 'pathname:///docs/intro' with baseUrl /baseUrl/", () => { + expect( + render(, {baseUrl: '/baseUrl/'}), + ).toMatchInlineSnapshot(` + + `); + }); + + it("can render 'pathname:///docs/intro' with target _self", () => { + expect(render()) + .toMatchInlineSnapshot(` + + `); + }); + + it("can render 'pathname:///docs/intro with trailingSlash: true", () => { + expect( + render(, {trailingSlash: true}), + ).toMatchInlineSnapshot(` + + `); + }); + }); + + describe('using "hash" router', () => { + const render = createLinkRenderer({router: 'hash'}); + + it("can render '/docs/intro'", () => { + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render '#anchor'", () => { + // It's important to use React Router link for hash router anchors + // See https://github.com/facebook/docusaurus/pull/10311 + expect(render()).toMatchInlineSnapshot(` + + `); + }); + + it("can render './relativeDoc'", () => { + // Not sure to remember exactly what's this edge case about + // still worth it to capture behavior in tests + expect(render()).toMatchInlineSnapshot(` + + `); + }); + }); +});