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

implemented a rule to forbid the export of symbols #1729

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Danil42Russia
Copy link
Contributor

@Danil42Russia Danil42Russia commented Feb 6, 2025

Issue

I'm writing a description. I created an MR to show the error.

<!-
Чтобы понять почему получилось такое решение, давайте я опишу проблемы, которые мне не дали сделать через TypeScript

  1. В проекте используется 8 версия eslint и старый способ конфигурации, из-за этого нельзя подключать плагины как модули JavaScript. Из-за этого нужно соблюдать правила именования пакетов.
  2. Из-за того что надо правильно именовать пакеты, то нужно создать новый пакет и подключить в данный проект. Создавать новый репозиторий излишне. Поэтому надо воспользоваться монорепой. Для этого существуют workspaces, но есть проблема. В проекте используется yarn 1 версии, и чтобы их начать использовать надо сделать проект приватным, но это не даст публиковать данный проект в npm репозиторий. Также yarn данный версии не даст подключить проект из workspaces в главный проект через синтакс workspace:^
  3. Из-за того, что не возможно воспользоваться подключением через workspaces, то можно пользоваться только подключением через file:/path/to/folder. Но это не симлинка, а копирование файлов из папки в node_modules во время установки пакетов
  4. Из-за копирования мы не можем сначала собрать наш линтер, допустим через TypeScript
    -->

Closes #1695.

Checklist

  • I have run the linter, formatter and spellchecker
  • I did not do unrelated and/or undiscussed refactorings

@Danil42Russia Danil42Russia requested a review from a team as a code owner February 6, 2025 20:12
@Danil42Russia Danil42Russia changed the title Danil42russia/eslint rules implemented a rule to forbid the export of symbols Feb 6, 2025
@@ -0,0 +1,6 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

A scary experiment, but we have #1045 and have to start somewhere...

@@ -0,0 +1,3 @@
const exampleSymbol = Symbol("example");
Copy link
Contributor

Choose a reason for hiding this comment

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

The primary reason I postponed doing this is that I remember there was an official template for eslint rule definitions, and I couldn't find it.

It required positive tests, negative tests and documentation, and also rules were implemented in TypeScript.

It's nice to have this error message in CI, but
a) we'll have to remove it
b) I also wonder whether it works with export const, and can't easily see it in tests

Finding/making the rule template is considerably more important :/

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.

Ban exporting witness Symbol
2 participants