uhyo / eslint-plugin-import-access

https://www.npmjs.com/package/eslint-plugin-import-access
MIT License
355 stars 9 forks source link

Forbid re-exports of non-importable variables #9

Closed mozisan closed 1 year ago

mozisan commented 1 year ago

連日でPRを投げて騒がしくしてしまいすみません :sob:

現時点で以下のような記述ができますが、

// Cannot import a private export 'subFoo'
import { subFoo } from "./sub/foo";

// No error
export { subFoo } from "./sub/foo";

「あるファイルからインポートできないものは再エクスポートもできない」という挙動の方がこのプラグインでガチガチに縛ろうとしたときには価値を発揮するかなと個人的には思ったのですが、いかがでしょうか・・・?

また、もしこの変更を導入する場合にはプラグインのオプションを追加してユーザーがオプトインできる形の方がいいのかも、ということもちょっと思ってはいますが、その辺ももしご意見あれば頂けると幸いでございます :pray: (別の観点として、プラグイン名としてはimport-accessの制御を担っており、こういったexport宣言についてのチェックは本来のスコープ外のようにも思えるので、その意味でもオプトインの方が好ましいのかなと思ったり)

uhyo commented 1 year ago

ありがとうございます! 基本的には import できないものを export { } from ...できるのはおかしいので、防げた方が望ましいと思います。これで壊れるコードもそこまで無さそうなのでオプトインにする必要も無さそうに思います。

ただ、考慮すべきパターンは結構ありそうです。

export {
  /**
   * @private
   */
  foo, // ←これはできるのか?
} from "..."

export * from "..." // これは全部チェックする必要がある
mozisan commented 1 year ago

こちらこそありがとうございます!!!!

export {
  /**
   * @private
   */
  foo, // ←これはできるのか?
} from "..."

これはTypeScript側の#47293が関連する点ですね。 手元のプロジェクトでこの記述が期待通りに動くか確認してみましたが、この差分が含まれる>=4.6のTypeScriptであれば動いていそうでした👍

export * from "..." // これは全部チェックする必要がある

これは結構ややこしそうですね🤒 このとき同時に以下のコードも全部チェックした方が良さそうでしょうか?

import * as Foo from "..."; // Foo.xで参照できるすべてがインポート可能であることをチェックする
uhyo commented 1 year ago

この差分が含まれる>=4.6のTypeScriptであれば動いていそうでした👍

それは朗報ですね。 🙂

import * as Fooのパターンも、抜け穴を全部防ぐという意味ではチェックすべきですね。 :sob:

ただ、全部一度にやる必要はないかと思います。協力いただいている立場なので、興味あるところをやっていただければと思います。 :pray:

mozisan commented 1 year ago

import * as Fooのパターンも、抜け穴を全部防ぐという意味ではチェックすべきですね。 😭 ただ、全部一度にやる必要はないかと思います。協力いただいている立場なので、興味あるところをやっていただければと思います。 🙏

承知しました! import * as Foo from "..."export * from "..."も結構また別の処理を追加しないといけなさそうな感じがするので、一旦そこは別PRで対応というように切り分けられたらと思います :pray: なので先にこのPRだけ提出という形にさせていただければと思うので、何か修正した方が良さそうな点などあればまたコメントいただければ修正します :bow: