-
Notifications
You must be signed in to change notification settings - Fork 38
add middleware to process external links substituting for ExternalLinksIntegration #1469
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
base: master
Are you sure you want to change the base?
add middleware to process external links substituting for ExternalLinksIntegration #1469
Conversation
🚀 Deployed on https://deploy-preview-1469--utelecon.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
↑間違えて承認しました……
方針を変えます。
手元の結果を貼っておきます。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
だいぶ筋が良さそうな気がしています。ありがとうございます!
diffを小さくしました。検証が容易になったと思います。
git checkout 3c1adc1fbd11d50d8b0db90c33db81d78f6e45dd
git checkout bcc461a5520aa2f7f05db5c3c35a16f7ad0d3a87 src/lib/ExternalLinksIntegration.ts
npx astro build --outDir dist_old
git switch -f 20250316_enable_rehype-external-link_in_development
npx astro build
diff -ruwB dist_old dist
|
approve していいような気もする vs リグレッションがあったらこわい |
何度もすみません、追加で修正をいれました。ExternalLinksIntegration利用時から以下の差分が生じます。
|
README.md
Outdated
@@ -114,7 +114,8 @@ Markdownファイルのフロントマターにかける設定は以下の通り | |||
- 外部リンクは,別タブで開くのが一般的です. | |||
- すべての外部リンクに対してこの属性を明示的に付与するのは冗長であり,また忘れる可能性も高いため,自動で付与すべきです. | |||
- 実装 | |||
- [`ExternalLinksIntegration.ts`](src/lib/ExternalLinksIntegration.ts)で実現しています.ここでは,ビルド後に全てのHTMLファイルをRehypeで改めてパースし,[`rehype-external-links`](https://github.com/rehypejs/rehype-external-links)を適用しています. | |||
- [Astroのミドルウェア機能](https://docs.astro.build/ja/guides/middleware/)を用いて[`externalLinks.ts`](src/middleware/externalLinks.ts)で実現しています.ここでは,アクセスしてきたユーザーに配信するHTMLをレスポンスの途中で一度Rehypeでパースし,[`rehype-external-links`](https://github.com/rehypejs/rehype-external-links)を適用してから送信するという動作によりページを処理しています. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここでは、……
これは微妙に偽で、実際はレンダリング時に後処理を噛ませているだけですね
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
そういうシミュレーションなのはわかるんですが、なんて言うといいですかね
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
作成された HTML に後処理を施します?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここでは,レンダリング後のHTMLを
dist
に保存する前に一度Rehypeでパースし,rehype-external-links
を適用するという動作によりページを処理しています.
にしました
src/middleware/externalLinks.ts
Outdated
const processor = unified() | ||
.use(rehypeParse) | ||
.use(rehypeExternalLinks, rehypeExternalLinksOptions) | ||
.use(rehypeStringify); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
今気づいたんですが、request ごとに processor を作ってて使い回してないんですね……。
かなり嬉しくないです。options.test が動的に作れなくなるのを考慮しても、1つを使い回すようにしたいです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
どうしますかね……モジュールレベルの状態作ります?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compare to f947016
案1
diff --git a/src/middleware/externalLinks.ts b/src/middleware/externalLinks.ts
index 1b0909cab..c20218783 100644
--- a/src/middleware/externalLinks.ts
+++ b/src/middleware/externalLinks.ts
@@ -1,4 +1,5 @@
import type { MiddlewareHandler } from "astro";
+import type { Element } from "hast";
import rehypeExternalLinks, {
type Options as RehypeExternalLinksOptions,
} from "rehype-external-links";
@@ -6,11 +7,30 @@ import rehypeParse from "rehype-parse";
import rehypeStringify from "rehype-stringify";
import { unified } from "unified";
+type Anchor = Element & { tagName: "a"; properties: { href: string } };
+
+function isAnchor(e: Element): e is Anchor {
+ return e.tagName === "a" && typeof e.properties.href === "string";
+}
+
+const state: { hostname?: string } = {};
+
const rehypeExternalLinksOptions: RehypeExternalLinksOptions = {
target: "_blank",
rel: ["noopener", "noreferrer"],
content: { type: "text", value: "" },
contentProperties: { className: ["external-link"] },
+ test: (e: Element) => {
+ if (!isAnchor(e)) {
+ // shall be unreachable
+ return true;
+ }
+ try {
+ return new URL(e.properties.href).hostname !== state.hostname;
+ } catch {
+ return true;
+ }
+ },
};
const processor = unified()
@@ -20,7 +40,12 @@ const processor = unified()
const mimeHtmlPattern = /^\s*text\/html(?:[\s;].*|$)/;
-const onRequest: MiddlewareHandler = async (_, next) => {
+export const onRequest: MiddlewareHandler = async (
+ { site: { hostname } = {} },
+ next
+) => {
+ state.hostname = hostname;
+
const response = await next();
if (response.headers.get("content-type")?.match(mimeHtmlPattern)) {
@@ -35,5 +60,3 @@ const onRequest: MiddlewareHandler = async (_, next) => {
return response;
};
-
-export default onRequest;
案2
diff --git a/src/middleware/externalLinks.ts b/src/middleware/externalLinks.ts
index 1b0909cab..c26979829 100644
--- a/src/middleware/externalLinks.ts
+++ b/src/middleware/externalLinks.ts
@@ -1,31 +1,63 @@
import type { MiddlewareHandler } from "astro";
-import rehypeExternalLinks, {
- type Options as RehypeExternalLinksOptions,
-} from "rehype-external-links";
+import type { Element, Root } from "hast";
+import rehypeExternalLinks from "rehype-external-links";
import rehypeParse from "rehype-parse";
import rehypeStringify from "rehype-stringify";
-import { unified } from "unified";
+import { unified, type Processor } from "unified";
-const rehypeExternalLinksOptions: RehypeExternalLinksOptions = {
- target: "_blank",
- rel: ["noopener", "noreferrer"],
- content: { type: "text", value: "" },
- contentProperties: { className: ["external-link"] },
-};
+type Anchor = Element & { tagName: "a"; properties: { href: string } };
+
+function isAnchor(e: Element): e is Anchor {
+ return e.tagName === "a" && typeof e.properties.href === "string";
+}
-const processor = unified()
- .use(rehypeParse)
- .use(rehypeExternalLinks, rehypeExternalLinksOptions)
- .use(rehypeStringify);
+class State {
+ hostname?: string;
+ private _processor: Processor<Root, Root, undefined, Root, string>;
+
+ constructor() {
+ this._processor = unified()
+ .use(rehypeParse)
+ .use(rehypeExternalLinks, {
+ target: "_blank",
+ rel: ["noopener", "noreferrer"],
+ content: { type: "text", value: "" },
+ contentProperties: { className: ["external-link"] },
+ test: (e: Element) => {
+ if (!isAnchor(e)) {
+ // shall be unreachable
+ return true;
+ }
+ try {
+ return new URL(e.properties.href).hostname !== this.hostname;
+ } catch {
+ return true;
+ }
+ },
+ })
+ .use(rehypeStringify);
+ }
+
+ get processor() {
+ return this._processor;
+ }
+}
+
+const state = new State();
const mimeHtmlPattern = /^\s*text\/html(?:[\s;].*|$)/;
-const onRequest: MiddlewareHandler = async (_, next) => {
+export const onRequest: MiddlewareHandler = async (
+ { site: { hostname } = {} },
+ next
+) => {
+ state.hostname = hostname;
+
const response = await next();
if (response.headers.get("content-type")?.match(mimeHtmlPattern)) {
return new Response(
- (await processor.process(await response.text())).value,
+ (await state.processor.process(await response.text())).value,
{
status: response.status,
headers: response.headers,
@@ -35,5 +67,3 @@ const onRequest: MiddlewareHandler = async (_, next) => {
return response;
};
-
-export default onRequest;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hostname チェックって必要でしょうか……?
むしろ hostname チェックは行わず、utelecon の絶対URLがあったら表示が変になって気付ける、のほうがいい気がしています。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
表示が変になって気付ける
現状気付けてないのがdevモードで気付けるようになる、という話ではありますね、たしかに
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ちなみに、「utelecon内部のページであっても外部リンクとして(別タブで)開かせたい場合は絶対URLを書くだけでよい」という暗黙知ができたとしても、あんまりネガティブには思わないですか?
rehypeプラグインを容易に追加できるMarkdownおよびMDXによるページに対し、developmentモード(
npm run dev
実行時)でも外部リンクのアイコンを表示するようにします。最終的にはすべてのHTMLファイルにアイコンを挿入する必要があるため、ビルド時(
npm run build
実行時)には既存のexternalLinks
インテグレーションは引き続き必要となります。