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(core): revert wrong anchor link implementation change #10311

Merged
merged 11 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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