Support moduleMainPaths #491
Conversation
src/ModuleManager.ts
Outdated
| // moduleMainScripts は将来的に非推奨となるため、moduleMainPaths を優先する | ||
| if (Object.keys(moduleMainPaths).length > 0) { | ||
| const regexp = new RegExp(`\/${path}\/`); | ||
| const targetPath = Object.values(moduleMainPaths).find(v => regexp.test(v)); |
There was a problem hiding this comment.
これ多分「 moduleMainScripts と同じところで処理して〜」と私が口走った気がしており申し訳ないんですが、
よく見ると直観に反する不要な処理になっているような気がしています。
moduleMainPathsの使い道がObject.keys().lengthとObject.values()しかない- key 側の値が全く見られていません
- もしこれが妥当なら、最初から moduleMainPaths は (values() の結果の) 配列であるべきに思えます
- 正規表現による判定が失敗する場合がある
"foo"に対して/\/foo\//が生成されますが、node_modules/foo/〜にも node_modules/bar/foo/〜` にもヒットするのでは
_resolvePath()のここで考慮したのに、これを呼び出す_require()でもう一度同じチェックが入っている
moduleMainPaths は「package.json のパスから index.js のパスへのテーブル」であることを考えると、ここでは処理できず、 むしろ _resolveAbsolutePathAsDirectory() の中で /package.json をつけた後の処理になるのではないでしょうか？
There was a problem hiding this comment.
ありがとうございます。_resolveAbsolutePathAsDirectory() の中で moduleMainPaths を参照するよう修正しました。
|
すいません、根本的な原因を失念していました。元の不具合を再現したコンテンツで動作確認取りました。 |
src/ModuleManager.ts
Outdated
| @@ -95,7 +96,8 @@ export class ModuleManager { | |||
| } | |||
|
|
|||
| // akashic-engine独自仕様: 対象の `path` が `moduleMainScripts` に指定されていたらそちらを参照する | |||
| if (moduleMainScripts[path]) { | |||
| // moduleMainScripts は将来的に非推奨となるため、moduleMainPaths が存在しない場合だけ参照する | |||
| if (Object.keys(moduleMainPaths).length === 0 && moduleMainScripts[path]) { | |||
There was a problem hiding this comment.
存在チェックのため Object.keys で判定しています。
There was a problem hiding this comment.
_moduleMainPaths: ModuleMainPathsMap | null にすればなくせませんか？ 頻発する処理でもないですが、処理としてですし記述もコンパクトになりそうです。
There was a problem hiding this comment.
型に null を追加し修正しました。
|
うーん、まだ考慮漏れがあることに気がつきました。「 とはいえ、これは akashic-engine 側の問題というよりは akashic install や scan の側でメンテすべきものだろうという点、akashic install 側がまだ --experimental つきオプションで実害がない点から、この PR は approve します。 |
このpull requestが解決する内容
[email protected] に追従し、
moduleMainPathsをサポートします。破壊的な変更を含んでいるか?