Skip to content
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

browserifyのpathやfsを使う際にOS依存のバグを埋め込んでしまうのを抑止したい #1245

Open
Hiroshiba opened this issue Mar 9, 2023 · 2 comments

Comments

@Hiroshiba
Copy link
Member

内容

nodeIntegrationがオフの環境で、レンダラープロセス内ではNode.jsのpathではなくbrowserifyのpathが自動的に使われます。
このpath-browserifyはwindows非対応なので、何も知らずに使うと気づかずにバグを埋め込みやすい気がします。

なにかバグを埋め込む前にバグの可能性気づける仕組みを作れないかな、というメモissueです。

今はwindows以外のOSでbasenameを取ってくるときと、書き出し先ディレクトリ固定状態でファイルパスをjoinするときにpath-browserifyが使われていそうです。

Pros 良くなる点

潜在的なバグに気づきやすそう

Cons 悪くなる点

実現方法

特定のファイル以外でpathライブラリを使うとエラーが出るようなlinterを作るとか・・・?
こんな感じを想像してます↓

うーん。。難しそう・・・?

@Hiroshiba Hiroshiba added 機能向上 要議論 実行する前に議論が必要そうなもの 優先度:低 labels Mar 9, 2023
@yamachu
Copy link
Member

yamachu commented Mar 9, 2023

lintでの解決をやるのであれば

https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-restricted-paths.md
例えばこの辺りのルールを使って、deny list方式でimportを禁止するのはありかもですね

allow list方式だと
https://github.com/knowledge-work/eslint-plugin-strict-dependencies
これがいい感じに機能した記憶があります

今回だとallowの方がマッチしてそうだなという印象はあります

@Hiroshiba
Copy link
Member Author

Hiroshiba commented Mar 9, 2023

allow形式なるほどです!!
pathUtilityみたいなのを作ってそこだけimport pathを許可って感じとか良さそう。

良さそうなので要議論タグを外します!

@Hiroshiba Hiroshiba removed the 要議論 実行する前に議論が必要そうなもの label Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants