Conflicting resolution priority between local theme and npm package

[u1f992]

themeに指定したディレクトリ名と同名のパッケージがnpmに存在する場合、ローカルのパッケージよりnpmのパッケージが優先されます。以下の例ではcssディレクトリが無視されcssがインストールされます。

> vivliostyle --version
cli: 8.15.0
core: 2.30.4

$ tree .
.
├── css
│   ├── package.json
│   └── style.css
├── manuscript.md
└── vivliostyle.config.js

vivliostyle.config.js

const vivliostyleConfig = {
  title: '_',
  author: '_',
  theme: 'css',
  entry: ['manuscript.md'],
  output: 'output.pdf',
  
  workspaceDir: '.vivliostyle',
};

module.exports = vivliostyleConfig;

css/package.json

{
    "name": "css",
    "main": "style.css"
}

$ cat .vivliostyle/themes/packages/css/Readme.md 
# css [![Build Status](https://travis-ci.org/reworkcss/css.svg?branch=master)](https://travis-ci.org/reworkcss/css)

CSS parser / stringifier.

## Installation
……

themeを./cssのように指定することで回避できますが、他フィールドがファイル名のみの表記で正常に処理される（manuscript.md、output.pdf）ことを鑑みると、一貫性を欠いた挙動に感じられます。また、非公開のテーマについてnpmに同名のパッケージがあるか確認することもまずないかと思います。この場合にはローカルのディレクトリ指定が優先されたほうが有用ではないでしょうか。

[spring-raining]

theme の名前からそのパッケージがnpm registryのものかローカル環境のものかを判定するロジックは @npmcli/arborist npm-package-arg というライブラリを用いており、静的に (npm registryを確認することなく) 実施されるよう設計されています。もし提案するように css という名前からローカル環境のパッケージを使用するような実装にする場合、

• npm registryに css というパッケージが存在するかを確認
• 存在しない、もしくはVivliostyle Themeパッケージでない場合、ローカル環境のパッケージを使用

というステップがビルド時に追加されることになり、相当な時間がビルド時間にかかってしまうことからそのままこの提案を受け入れることは難しいです。
ただ、npm packageのインポートに失敗したときに同名のディレクトリがローカル環境にあった場合に、./css という名前に変更するようエラーメッセージとして提案する、といった改善は良いかと思います。

[spring-raining]

なお、 foo/bar のように名前が / が含まれており、かつ名前の先頭が @ で始まらない場合は判定結果に関わらずローカル環境のパッケージを使用するようになっています。参考: #373

[u1f992]

ありがとうございます。おっしゃる通りレジストリを見に行くのは実用的ではなさそうです。
しかし村上さんの「最後のスラッシュがなくても、そのディレクトリがある場合は、ディレクトリの指定として扱われるべきでしょう。」という発言からはいっそう、theme: 'css'と指定されていればcssディレクトリを見に行くべきだと感じられます……。

• npm registryにcssというパッケージが存在するかを確認
• 存在しない、もしくはVivliostyle Themeパッケージでない場合、ローカル環境のパッケージを使用

この実装案が少しわからなかったのですが、素朴な案として「npm-package-argにかける前に、パスとして解釈して存在するか・Vivliostyle Themeパッケージか判別する」処理を挟むのは不都合がありそうでしょうか？

[spring-raining]

theme はあくまでnpm packageを指定するためのオプションのため、npmの名前解決のルールに例外的な処理を加えることで将来的なバグの原因になることを懸念しています。
また、ディレクトリの有無で動作が変わるというのは利用者側としてもあまり直感的ではないように思います。もし、css というディレクトリを指定しようとしたものの実際にはそのディレクトリがなかった場合、ディレクトリが存在しないというエラーではなくnpm registryに css というパッケージが存在しないというエラーになり、原因の理解が難しくなりそうです。それよりは、(npmと同様のルールで) 明示的にローカルパッケージであることを示す必要があることを周知するほうが良いのではないでしょうか？

[u1f992]

theme はあくまでnpm packageを指定するためのオプションのため、npmの名前解決のルールに例外的な処理を加えることで将来的なバグの原因になることを懸念しています。

正直なところ、バグの原因になりそうなのは同意します。

npm install ~の指定と同じルールと考えると理屈はわかりますが、カレントディレクトリ直下の場合のみ接頭辞的に./が必要なのは、現状のvivliostyle.config.jsにある// .css or local dir or npm package.からは読み取れないように思います。文字通りには、先にディレクトリ名として考慮してくれることを期待します。実装は変えずこの説明を正確にするのでもよいと思います。

実装を変える場合、カレントディレクトリのfooというディレクトリを意図してtheme: "foo"と書いたが実際にはfooディレクトリが存在しないかつnpmレジストリにも存在しない場合には、「npm registryにfooというパッケージが存在しない」というエラーではなく、「ディレクトリも、レジストリにパッケージも存在しない」というエラーを出せば原因の特定はむしろ容易になりそうです。実現可能かはわかりません

flowchart LR
    s0{"そのディレクトリにnpmパッケージが存在する？"}
    s1{"（既存の処理）"}
    s2["パッケージを使用する"]
    s3["エラー「ディレクトリも、レジストリにパッケージも存在しない」"]
    
    s0 -- yes --> s2
    s0 -- no --> s1
    s1 -- yes --> s2
    s1 -- no --> s3

Loading

レジストリにパッケージがないケースは現状でもインストールに失敗するのでまだよいほうです。
カレントディレクトリ直下で管理するようなワンオフのテーマに凝った名前は付けない（css/、theme/等）と仮定すると、このような汎用的な名前はたいていすでにレジストリにVivliostyleテーマではないパッケージが存在し、特にエラーもなく無関係のパッケージをインストールしてしまい正しくビルドされなかったり意図したスタイルが当たらない状態になります。こうなると先に提示していただいた"foo" → "./foo"への変更提案も難しそうに見えます。

[spring-raining]

カレントディレクトリ直下で管理するようなワンオフのテーマに凝った名前は付けない（css/、theme/等）と仮定すると、このような汎用的な名前はたいていすでにレジストリにVivliostyleテーマではないパッケージが存在し、特にエラーもなく無関係のパッケージをインストールしてしまい正しくビルドされなかったり意図したスタイルが当たらない状態になります。こうなると先に提示していただいた"foo" → "./foo"への変更提案も難しそうに見えます。

完全な解決策ではありませんが、インストールしたパッケージがVivliostyle Themeに準拠しているかどうかを判定することである程度は解決可能です。例えば、インストールしたパッケージのpackage.jsonに vivliostyle フィールドがあるかどうかをチェックするなどで判定できます。

[u1f992]

実装を複雑にしたくないのは十分理解いたします。themeの解決方法自体は変えずに、themeに何を記述できるのかのドキュメントを補う形で進めていければよいのかなと思っています。現状想定されているのは

• *.css URLおよびローカルファイルへのパス
• ローカルのnpmパッケージへのパス（ただし/を含む）
• npm install ～相当のパッケージ指定

という感じでしょうか。

インストールしたパッケージのpackage.jsonにvivliostyleフィールドがあるかどうかをチェックするなどで判定できます。

パッケージがVivliostyleテーマかどうか判定できるようにするのはこの件に限らず役に立つかもしれませんが、既存のテーマへの影響が気にかかります。

[MurakamiShinyu]

私は #524 (comment) の案のローカルにnpmパッケージ（package.json が存在）があればそれを優先するというのが分かりやすいと思います。ディレクトリの存在だけではなく package.json の存在もチェックすることで、公開テーマの利用を意図しているのに、たまたま同名のディレクトリがローカルにあるために、意図した動作にならなくなるということは防げると思います。