feat: Implement htmlProcessor as the documentProcessor for HTML#734
feat: Implement htmlProcessor as the documentProcessor for HTML#734u1f992 wants to merge 4 commits into
htmlProcessor as the documentProcessor for HTML#734Conversation
aa1bba6 to
dea7ddf
Compare
1561b6d to
c194a2d
Compare
c194a2d to
9c531b3
Compare
|
実装は普通だと思うのですが、documentProcessorと同じバージョンのProcessorを公開するために、メイン部分と干渉する別バージョンのパッケージを別名でインストールしています。src/processor/html-processor.tsの冒頭コメントにより詳しく残してあります。 |
|
examples/customize-processorを更新したところ既存のコードが型チェックを通っていなかったので修正しました。rehype-expressive-codeはunified@10に依存しているため正常に解決する方法はないのですが、同じunist@2の範囲ですし実際動いているので多分大丈夫とみてアサーションで回避しています。また |
spring-raining
left a comment
There was a problem hiding this comment.
レビューが遅くなりすみません 🙇 こ確認ください!
| content = getJsdomFromString({ | ||
| html: String(vfile), | ||
| contentType: source.contentType, | ||
| }); |
There was a problem hiding this comment.
こちらの変更により既存のHTML processorにrehypeのパース処理が加わることになりますが、この変更はこれまでJSDOMでは読み込めていたHTMLファイルがrehypeのパースで失敗する可能性があり、ややリスクの高い変更のように思います。
vivliostyle-cliが内部でrehype processorを使用する方法の代わりに、単純な文字列として(X)HTMLを受け渡すなど、より汎用的なオプションを検討してもらえないでしょうか?
function serializeJSDOM(content: jsdom.JSDOM) {
if (content.window.document.contentType === 'application/xhtml+xml') {
return `${XML_DECLARATION}\n${serializeToXml(content.window.document)}`;
} else {
return content.serialize();
}
}
...
content = await getJsdomFromUrlOrFile({ src: source.pathname });
if (source.htmlProcessor) {
const processedHtml = await source.htmlProcessor(serializeJsdom(content), ...);
content = getJsdomFromString({
html: processedHtml,
contentType: source.contentType,
});
} else {
content = await processManuscriptHtml(content, ...);
}There was a problem hiding this comment.
JSDOMとrehypeはどちらも最終的にparse5を使用するはずなので、JSDOMでパースできてrehypeでパースできないHTMLは基本的には考えられないかと思います。XHTMLについてはそうではないのでリスクはあると思います。
htmlProcessorをシリアライズした文字列に対して実行する形にするのは、
documentProcessorなど既存の方式と一貫性がない- ユーザー側が何らかのデシリアライズ処理を用意しなければならない
- デシリアライズ→シリアライズを合計で3回行うことになり明らかに非効率
などから避けたいです。
プラグインのために複数回のシリアライズを検討したり一貫性の問題が生じるのは、そもそも文書変形にunifiedとJSDOMの2系統が混在しているためかと思います。現在のPRの方式は、HTML処理のまず入口部分をunifiedベースに移行し、混在に対処しようとしているものです。documentProcessorと一貫性のない形なら入れる意味がないと思っています。いかがでしょうか。
There was a problem hiding this comment.
すみません、性急に進めるつもりはなかったのですが、この変更には説明できなかった追加の意図があります。
しばらく文書処理部分の拡張を進めてきた結果、内部の文書処理にJSDOMを使うのはブラウザAPI互換で書ける程度のメリットしかなく、node:vmのオーバーヘッドによるパフォーマンス上の不都合が無視できないという認識に至っています。これらはすべて、rehype/hast処理としてより合理的に書くことができます。
こちらがこのPRから進めたPoCで、内部処理からJSDOMを排除したものです(Claude Codeで強引に進めたものです。PRとしては別途書き直します)。予想通り、examplesの全PDFビルドで20%近くのパフォーマンス改善が可能です。例示のための各数個の原稿ファイルで得られる改善がこの量なので、実際の書籍で原稿ファイルが増えればより大きな改善が見込めるはずです。
その足がかりとしてまずHTML入力部分のunified化を行いたいという思惑があります。マイナーバージョンアップで済む、意味が変わらないテストケースの調整だけで通せるようにしていますが、厳密に動作の互換性を見るなら次のメジャーバージョンでの対応でも問題ないと考えています。何とぞご検討いただけますか?
There was a problem hiding this comment.
ご説明ありがとうございます。rehype導入の意図について理解しました。
ただ、現時点では将来的にJSDOMの使用を完全に廃止することは難しいと考えています。例えば、vivliostyle-cliでは 入力としてWeb上のリソースを指定して その内容をWebpubやEPUBとして変換する機能がありますが、この機能はJSDOMの fromURL APIにより実現しており、JSDOMを一種のクローラーとして動作させることでこの機能を実現しています。
もちろん今回対象としている処理とWeb上のリソースは全く違うものです。しかし、JSDOMを通して元データを抽象化することで (X) HTMLの処理をシンプルに保っている側面もあるため、JSDOMとrehype両方の実装を維持することはメンテナンスコストを払うだけの強いメリットがない限り難しい判断です。documentProcessor のカスタマイズの需要が不明なこと、Vivliostyle.js自体の処理時間が支配的なことを考えると、速度面のメリットだけでは採用しづらいと考えています。
There was a problem hiding this comment.
すいません、今回の変更は documentProcessor の設定に限らず適用されるものなので、HTMLの入力に関しては一律で高速化されるものですね。であれば、確かに高速化の対象としてJSDOMを介さないケースを用意することは意義がありそうです。
ただ、いずれにしても (remarkを内部に持つVFMとは異なり) rehypeがHTMLのプロセッサーとしてふさわしいかは議論すべき箇所だと思うので、文字列やbufferなどの標準的なデータ以外をAPIのインターフェースとして採用するのはもう少し考えたいです。
There was a problem hiding this comment.
承知しました。プロダクトとしての舵取りが必要だと思います。こちらは要検討で保留ということでお願いします。一旦draftにしておきます。
JSDOMのサブリソース追跡についてもある程度調べていて、多くのケースでは容易に代替実装できるものの、現在行われる処理のうち特にCSSのimportの追跡には追加の依存ライブラリ(具体的にはPostCSS)を使いたいところです。上のフォークにコメントを追加しておきました。
ただこの点はむしろ、JSDOMに中途半端に乗りかかるよりもVivliostyle CLIが自前で明示的に処理すべきだろうと思います。たとえば現在はimg要素のリモートリソースは収集しますがCSS内のurl()の画像は収集しません。JSDOMの都合による挙動がEPUBとして妥当なのかは疑問です。
単に私が慣れたというところは否定しないのですが、unified/unist(hast・mdast)は速度面だけでなく、
- feat: Allow custom extensions #667 (comment) で一度検討した、マッチャーによるProcessor選択に統合できる
- ツールが純JavaScriptで用意されているためWebや他JSランタイムでも動作できる
- Vivliostyle Viewer should support Markdown documents vivliostyle.js#1506 の発展形として、Vivliostyle CLIの機能の一部をVivliostyle.jsやPubと共用を検討できる
- JSONへのシリアライズが考慮されており他言語との相互運用も現実的
- unifiedの作者はRustでMarkdownやMDXの再実装を行なっていますが、unistの仕様はこちらでも引き続き使用されています
など利点があります。また@ napi-rs/canvasに限らずネイティブコード依存は根本的な対処が難しい問題(例: #697 )を発生させることも考慮の材料になるのではと思います。
よろしくご検討ください。
closes: #733