Skip to content

Commit

Permalink
fix(core): revert wrong anchor link implementation change (#10311)
Browse files Browse the repository at this point in the history
  • Loading branch information
slorber committed Jul 19, 2024
1 parent 61d6858 commit 868d72f
Show file tree
Hide file tree
Showing 3 changed files with 371 additions and 6 deletions.
10 changes: 6 additions & 4 deletions .github/workflows/build-perf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?
20 changes: 18 additions & 2 deletions packages/docusaurus/src/client/exports/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -157,7 +157,15 @@ function Link(
const hasInternalTarget = !props.target || props.target === '_self';

// Should we use a regular <a> 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 <a> 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!);
Expand All @@ -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
<a
Expand All @@ -175,6 +189,7 @@ function Link(
{...(targetLinkUnprefixed &&
!isInternal && {target: '_blank', rel: 'noopener noreferrer'})}
{...props}
{...testOnlyProps}
/>
) : (
<LinkComponent
Expand All @@ -186,6 +201,7 @@ function Link(
// Avoid "React does not recognize the `activeClassName` prop on a DOM
// element"
{...(isNavLink && {isActive, activeClassName})}
{...testOnlyProps}
/>
);
}
Expand Down
Loading

0 comments on commit 868d72f

Please sign in to comment.