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

Forbid re-exports of non-importable variables #9

Merged

Conversation

mozisan
Copy link
Contributor

@mozisan mozisan commented May 31, 2023

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

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

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

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

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

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

@uhyo
Copy link
Owner

uhyo commented Jun 1, 2023

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

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

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

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

@mozisan
Copy link
Contributor Author

mozisan commented Jun 2, 2023

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

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

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

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

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

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

@uhyo
Copy link
Owner

uhyo commented Jun 2, 2023

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

それは朗報ですね。 🙂

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

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

@mozisan mozisan marked this pull request as ready for review June 3, 2023 16:21
@mozisan
Copy link
Contributor Author

mozisan commented Jun 3, 2023

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

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

Copy link
Owner

@uhyo uhyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます! 🥰

@uhyo uhyo merged commit f98670d into uhyo:master Jun 4, 2023
@mozisan mozisan deleted the fix/forbid-reexports-of-non-importable-variables branch June 4, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants