otoyo / astro-notion-blog

🚀 Begin building your very own Notion Blog with Astro.
https://astro-notion-blog.pages.dev/
MIT License
766 stars 499 forks source link

Fixed getNavLink to follow trailingSlash:never in astro.config.mjs. #187

Closed chuua closed 9 months ago

chuua commented 9 months ago

サブパスで運用する際にastro側でTrailing Slashを非表示にすると一部でリンク切れが発生するため、 astro.config.mjsのtrailingSlash: 'never'へ追従する修正案になります。

再現方法

  1. /src/server-constrants.tsのBASE_PATHを設定 (ここでは仮に/blog/とします。)
  2. astro.config.mjsのdefineConfigにてtrailingSlash: 'never'を設定
  3. astroは/blog、/src/lib/blog-helpers.tsのgetNavLink()は/blog/を前提とした不整合が発生

確認したリンク切れ発生個所

vercel[bot] commented 9 months ago

Someone is attempting to deploy a commit to a Personal Account owned by @otoyo on Vercel.

@otoyo first needs to authorize it.

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
astro-notion-blog ❌ Failed (Inspect) Jan 14, 2024 11:57am
chuua commented 9 months ago

確認が甘くお手数おかけしました! npm run devでは参照できたため、Cloudflare PagesがFailedしている事を見逃していました...。

astro.config.mjsをビルド後の環境から参照する方法は実現が厳しそうだったため、 BASE_PATH等と同じようにserver-constrants.tsへTRAILING_SLASHを設定し参照する形へ変更しました。

astroもastro-notion-blogも触り始めたばかりで手探り感が否めませんが、 もしよろしければサブパス対応の一環として取り込んでいただけたらなと思います。

otoyo commented 9 months ago

確認なのですが trailingSlash: never を指定したいユースケースは何でしょうか?デフォルトで ignorant なので末尾スラッシュの有無に関係なくどちらにもマッチします。 https://docs.astro.build/en/reference/configuration-reference/#trailingslash

chuua commented 9 months ago

確認なのですが trailingSlash: never を指定したいユースケースは何でしょうか?デフォルトで ignorant なので末尾スラッシュの有無に関係なくどちらにもマッチします。 https://docs.astro.build/en/reference/configuration-reference/#trailingslash

デフォルトの動作について、ありがとうございます。

trailingSlash: neverを指定したいユースケースですが、 同一ドメイン上の他のサブパスで既にtrailingSlashの無い状態で稼働しているものもあり、 ドメイン全体でサブパス1段目に統一感を持たせたいと思った事がきっかけでした。

はじめはNext.jsの動きを真似て/なしへのリダイレクトを試したのですが(Cloudflare / フレームワークのRouter等) html内のCanonicalタグが/ありのままになってしまう問題が残ることに...。 (プロダクトの出自もあると思いますが、Next.jsは/ありからなしへリダイレクトがデフォルトの様でした。) https://nextjs.org/docs/app/api-reference/next-config-js/trailingSlash

その後、astro自体にもtrailingSlashの設定がある事が分かったため変更してみたところ、 getNavLinkから生成されるパスがリンク切れを起こすことが分かってきたので 「もしastroへの設定に合わせて上記パスも切り替わると便利そうだ」と思いPR発行させていただいた流れとなります。 ただ実際には考慮漏れも多く、issueにて報告・エッジケース過ぎないかの判断や 実装はotoyoさんにお願いした方がお手間が少なかったかもしれません。。

otoyo commented 9 months ago

ユースケースのご説明ありがとうございます。よく理解できました。 trailingSlashの設定を環境変数で持たせることで要件は満たせそうですが、ベースとしているフレームワークの設定をastro-notion-blogのユーザーに強いることになるので、それは最終手段にしたいと思っています。

下記のAstroのドキュメントによれば、import.meta.env.BASE_URL は config の basetrailingSlash 両方が反映されるとのことなので、こちらで軸に私の方で差分を作ってみようと思います。 https://docs.astro.build/en/reference/configuration-reference/#base

chuua commented 9 months ago

ベースとしているフレームワークの設定をastro-notion-blogのユーザーに強いることになる

仰る通り、方法が安直でした。。

下記のAstroのドキュメントによれば、import.meta.env.BASE_URL は config の base と trailingSlash 両方が反映されるとのことなので、こちらで軸に私の方で差分を作ってみようと思います。 https://docs.astro.build/en/reference/configuration-reference/#base

ありがとうございます! 自分も微力ながら模索を続けたいと思います。 お手数おかけいたしますが、引き続きよろしくお願いいたします。

otoyo commented 9 months ago

すみません、思い込みで話を進めてしまっていたのですが、前提として astro-notion-blog は全てのページを静的ファイルとして扱うため、本番環境のURLは build.format によってのみ決定されます。 https://docs.astro.build/en/reference/configuration-reference/#buildformat

trailingSlash は説明にある通りローカル環境(開発環境)でのみ有効なオプションです。

Set the route matching behavior of the dev server. Choose from the following options:

https://docs.astro.build/en/reference/configuration-reference/#trailingslash

まとめると、本番環境で末尾スラッシュなしという設定は全てを静的ファイルとして扱う astro-notion-blog では不可能です。もしやるとすればCloudflareなどホスティングサービス側でURLリライティング等の機能を使う必要があります。

otoyo commented 9 months ago

この Issue はクローズしますが、質問あればお気軽にコメントしてください。

chuua commented 9 months ago

なるほどです。 build.formatとCloudflare Pagesの特性を調整すると見かけ上のURLはどうにかなりそうな気配がしております。 https://developers.cloudflare.com/pages/configuration/serving-pages/#route-matching

それでうまく動かなくなってしまう箇所やビルド後のソースで/が付いてしまう所(canonical等)は 個人カスタマイズの範疇で楽しみたいと思います。

お時間割いてご検討いただき、ありがとうございました! 引き続きastro-notion-blogを使わせていただき、 不具合報告や協力できる事がありましたら、よろしくお願いいたします。

otoyo commented 9 months ago

build.format: file と Route Matching を組み合わせると実現できそうですね。 もしうまくできたらぜひブログに書いてこちらでも教えてください。