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

feat: add externalLinkPrefixes support #1674

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions packages/core/src/node/mdx/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export async function createMDXOptions(
remarkPluginNormalizeLink,
{
base: config?.base || '',
externalLinkPrefixes: config?.externalLinkPrefixes,
cleanUrls,
defaultLang,
root: docDirectory,
Expand Down
8 changes: 3 additions & 5 deletions packages/core/src/node/mdx/remarkPlugins/checkDeadLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import path from 'node:path';
import { visit } from 'unist-util-visit';
import type { Plugin } from 'unified';
import { logger } from '@rspress/shared/logger';
import { cleanUrl, isProduction } from '@rspress/shared';
import { cleanUrl, isExternalUrl, isProduction } from '@rspress/shared';
import type { RouteService } from '@/node/route/RouteService';
import { normalizePath } from '@/node/utils';

Expand All @@ -12,8 +12,6 @@ export interface DeadLinkCheckOptions {
routeService: RouteService;
}

const IGNORE_REGEXP = /^(https?|mailto|tel|#)/;

export function checkLinks(
links: string[],
filepath: string,
Expand All @@ -22,7 +20,7 @@ export function checkLinks(
) {
const errorInfos: string[] = [];
links
.filter(link => !IGNORE_REGEXP.test(link))
.filter(link => !isExternalUrl(link, routeService.externalLinkPrefixes))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some differences here. # is missed, #my-anchor should not be considered as externalUrl but can be ignored in dead links check.

.map(link => normalizePath(link))
.forEach(link => {
const relativePath = path.relative(root, filepath);
Expand Down Expand Up @@ -64,7 +62,7 @@ export const remarkCheckDeadLinks: Plugin<
return;
}

if (!url.startsWith('http') && !url.startsWith('https')) {
if (!isExternalUrl(url, routeService.externalLinkPrefixes)) {
const { routePath: normalizeUrl } = routeService.normalizeRoutePath(
// fix: windows path
url.split(path.sep).join('/')?.split('#')[0],
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/node/mdx/remarkPlugins/normalizeLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,17 @@ interface LinkNode {
* Remark plugin to normalize a link href
*/
export const remarkPluginNormalizeLink: Plugin<
[{ base: string; root: string; cleanUrls: boolean }],
[
{
base: string;
externalLinkPrefixes?: string[];
root: string;
cleanUrls: boolean;
},
],
Root
> =
({ base, root, cleanUrls }) =>
({ base, externalLinkPrefixes, root, cleanUrls }) =>
(tree, file) => {
const images: MdxjsEsm[] = [];
visit(
Expand All @@ -36,7 +43,7 @@ export const remarkPluginNormalizeLink: Plugin<
// eslint-disable-next-line prefer-const
let { url, hash } = parseUrl(node.url);

if (isExternalUrl(url)) {
if (isExternalUrl(url, externalLinkPrefixes)) {
node.url = url + (hash ? `#${hash}` : '');
return;
}
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/node/route/RouteService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ export class RouteService {

#pluginDriver: PluginDriver;

#externalLinkPrefixes?: string[];

get externalLinkPrefixes() {
return this.#externalLinkPrefixes;
}

constructor(
scanDir: string,
userConfig: UserConfig,
Expand All @@ -128,6 +134,7 @@ export class RouteService {
[]
).map(item => item.lang);
this.#base = userConfig?.base || '';
this.#externalLinkPrefixes = userConfig?.externalLinkPrefixes;
this.#tempDir = tempDir;
this.#pluginDriver = pluginDriver;

Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/node/runtimeModule/siteData/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,14 @@ export async function siteDataVMPlugin(context: FactoryContext) {
);

const siteData: SiteData = {
root: userDocRoot,
title: userConfig?.title || '',
description: userConfig?.description || '',
icon: userConfig?.icon || '',
route: userConfig?.route,
themeConfig: normalizeThemeConfig(userConfig, pages),
base: userConfig?.base || '/',
externalLinkPrefixes: userConfig?.externalLinkPrefixes || [],
lang: userConfig?.lang || '',
locales: userConfig?.locales || userConfig.themeConfig?.locales || [],
logo: userConfig?.logo || '',
Expand Down
8 changes: 3 additions & 5 deletions packages/shared/src/runtime-utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,10 @@ export function normalizeSlash(url: string) {
return removeTrailingSlash(addLeadingSlash(normalizePosixPath(url)));
}

export function isExternalUrl(url = '') {
export function isExternalUrl(url = '', externalLinkPrefixes?: string[]) {
return (
url.startsWith('http://') ||
url.startsWith('https://') ||
url.startsWith('mailto:') ||
url.startsWith('tel:')
/^((https?|mailto|tel):)?\/\//.test(url) ||
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this regex is not as same as before, you can see tests failed in CI. And this regex has edge cases like //www.example.com and for long urls, the performance is not very good for long and complex urls.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend to keep origin methods to make it more readable and easier to understand for simple checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//www.example.com should not be considered as external?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just take an example to show the difference.

externalLinkPrefixes?.some(prefix => url.startsWith(prefix))
);
}

Expand Down
5 changes: 5 additions & 0 deletions packages/shared/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export interface UserConfig<ThemeConfig = DefaultThemeConfig> {
* @default '/'
*/
base?: string;
/**
* Recognize the links with following prefixes as external links.
*/
externalLinkPrefixes?: string[];
/**
* Path to html icon file.
*/
Expand Down Expand Up @@ -195,6 +199,7 @@ export type BaseRuntimePageInfo = Omit<
export interface SiteData<ThemeConfig = NormalizedDefaultThemeConfig> {
root: string;
base: string;
externalLinkPrefixes: string[];
lang: string;
route: RouteOptions;
locales: { lang: string; label: string }[];
Expand Down
4 changes: 3 additions & 1 deletion packages/theme-default/src/components/Link/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
normalizeRoutePath,
withBase,
isEqualPath,
usePageData,
} from '@rspress/runtime';
import nprogress from 'nprogress';
import { routes } from 'virtual-routes';
Expand Down Expand Up @@ -35,7 +36,8 @@ export function Link(props: LinkProps) {
keepCurrentParams = false,
...rest
} = props;
const isExternal = isExternalUrl(href);
const { siteData } = usePageData();
const isExternal = isExternalUrl(href, siteData.externalLinkPrefixes);
const target = isExternal ? '_blank' : '';
const rel = isExternal ? 'noopener noreferrer' : undefined;
const withBaseUrl = isExternal ? href : withBase(normalizeHref(href));
Expand Down
Loading