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

LINT: Resolve import-order/experimental (Need your feedback!) #85

Open
azinit opened this issue Feb 2, 2022 · 20 comments
Open

LINT: Resolve import-order/experimental (Need your feedback!) #85

azinit opened this issue Feb 2, 2022 · 20 comments
Labels
help wanted Extra attention is needed

Comments

@azinit
Copy link
Member

azinit commented Feb 2, 2022

Were added by #82 for import/order rule by @Krakazybik

Should be merged with base config, or deleted at all

Why experimental?

#75 (comment)
#75 (comment)

Variant 1: import-order/experimental

With spaces and reverted order ("from abstract - to specific")

import axios from "axios";                           // 1) external libs

import { debounce } from "shared/lib/fp";            // 2.1) Layers: shared
import { Button } from "shared/ui";                  // 2.1) Layers: shared

import { authModel } from "entities/auth";           // 2.2) Layers: entities
import { commentModel } from "entities/comment";     // 2.2) Layers: entities
import { postModel } from "entities/post";           // 2.2) Layers: entities

import { LoginForm } from "features/login-form";     // 2.3) Layers: features

import { Header } from "widgets/header";             // 2.4) Layers: widgets

import { data } from "../fixtures";                  // 3) parent
import { getSmth } from "./lib";                     // 4) sibling

Variant 2: import-order (base)

import axios from "axios";                           // 1) external libs
import { Header } from "widgets/header";             // 2.1) Layers: widgets
import { LoginForm } from "features/login-form";     // 2.2) Layers: features
import { authModel } from "entities/auth";           // 2.3) Layers: entities
import { commentModel } from "entities/comment";     // 2.2) Layers: entities
import { postModel } from "entities/post";           // 2.3) Layers: entities
import { Button } from "shared/ui";                  // 2.4) Layers: shared
import { debounce } from "shared/lib/fp";            // 2.4) Layers: shared
import { data } from "../fixtures";                  // 3) parent
import { getSmth } from "./lib";                     // 4) sibling

Please, leave your vote below:

"👍" - if you prefer to use import-order/experimental at stable config
"👎" - if you prefer to use import-order at stable config
"👀" - if you aren't sure

Feel free leave below your comments

@azinit azinit changed the title LINT: Resolve import-order/experimental LINT: Resolve import-order/experimental (Need your feedback!) Feb 2, 2022
@azinit azinit pinned this issue Feb 2, 2022
@azinit azinit added the help wanted Extra attention is needed label Feb 2, 2022
@azinit
Copy link
Member Author

azinit commented Feb 2, 2022

@feature-sliced/core @feature-sliced/contributors please, leave your feedback

@PUndef
Copy link

PUndef commented Feb 2, 2022

В первом варианте мне нравится что порядок слоёв такой, какой должен быть по методологии. Однако нет разделения групп, это достаточно удобная опция, по крайней мере для меня

@SQReder
Copy link

SQReder commented Feb 2, 2022

Плюсую разделение групп, как стандарт

@PUndef
Copy link

PUndef commented Feb 2, 2022

В рамках своего предложения Я говорю о такого рода разделении (чтобы не было недопонимания):

import axios from "axios";                           // 1) external libs

import { Header } from "widgets/header";             // 2.1) Layers: widgets

import { LoginForm } from "features/login-form";     // 2.2) Layers: features

import { authModel } from "entities/auth";           // 2.3) Layers: entities
import { commentModel } from "entities/comment";     // 2.2) Layers: entities
import { postModel } from "entities/post";           // 2.3) Layers: entities

import { Button } from "shared/ui";                  // 2.4) Layers: shared
import { debounce } from "shared/lib/fp";            // 2.4) Layers: shared

import { data } from "../fixtures";                  // 3) parent
import { getSmth } from "./lib";                     // 4) sibling

@PUndef
Copy link

PUndef commented Feb 2, 2022

но вроде как некоторые топят за такой вариант. в целом Я тоже не против такого

import axios from "axios";                           // 1) external libs

import { Header } from "widgets/header";             // 2.1) Layers: widgets
import { LoginForm } from "features/login-form";     // 2.2) Layers: features
import { authModel } from "entities/auth";           // 2.3) Layers: entities
import { commentModel } from "entities/comment";     // 2.2) Layers: entities
import { postModel } from "entities/post";           // 2.3) Layers: entities
import { Button } from "shared/ui";                  // 2.4) Layers: shared
import { debounce } from "shared/lib/fp";            // 2.4) Layers: shared

import { data } from "../fixtures";                  // 3) parent
import { getSmth } from "./lib";                     // 4) sibling

@azinit
Copy link
Member Author

azinit commented Feb 2, 2022

Поменял порядок, а то многие путают)
(сами пункты по существу не поменялись, эмодзи актуальны)

Еще раз:

👍 - за экспериментальный конфиг как стейбл
👎 - за базовый конфиг (без экспериментов) как стейбл

@Kelin2025
Copy link

Писал в треде, продублирую и сюда

TL;DR - считаю второй вариант нелогичным, а путь app -> shared в доке FSD исторической ошибкой, которую надо исправлять


В своих проектах я использую порядок npm пакеты -> shared -> entities -> features -> ... -> локальные
Аргументация элементарная:

  • NPM-пакеты - самый недоступный и ультимативно абстрагированный уровень
  • Shared - тут помимо UI и API тоже лежат локальные библиотеки, смысл схожий с пакетами
  • Дальше, соответственно, все слои по порядку
  • Пока не спустимся до локального слоя

Обратный порядок же превратит импорты в лютую кашу.
Предположим, что у нас страница. Будет следующее:

import axios from 'axios'             // Самый топ-левел (Возьмём его за 0)
import { Foo } from '@/widgets/foo'   // Виджеты (4)
import { Bar } from '@/features/bar'  // Фичи (3)
import { Bar } from '@/entities/baz'  // Сущности (2)
import { Button } from '@/shared/ui'  // Shared (1)
import { Smth } from './ui/Smth'      // Локальное от страницы (5)

И шо это за дичь?)) Как с американским форматом дат история выходит. Кому-то привычно, но для остальных - диссонанс.


Более того, мне не ясно, почему в методологии вообще сложился путь от app к shared. Такое направление с семантической точки зрения некорректно, потому что ты импортируешь из shared в сущности итд, а не наоборот.
Можно сказать "это путь зависимостей, мол, стрелка значит, что страница зависит от виджетов, фич итд". Однако, направление стрелки всё ещё будет некорректным, поскольку это зависимости вставляются в страницу, а не страница в них.
Здесь же складывается ощущение, что у нас библиотека сначала используется, а потом объявляется.


Если уж хочется - то можно это сделать опцией, но лично я категорически против принимать app -> shared за дефолтное поведение, это совершенно не логично.
А то, что в доке FSD написан такой порядок - это апелляция к доке, а не аргумент.

@unordinarity
Copy link
Member

Упрощаю тезис @Kelin2025: импорты сортируются от глобальных к локальным, то есть блок импортов из слоёв должен быть отсортирован от shared к app, не наоборот.

@unordinarity
Copy link
Member

Разделение импортов из слоёв по блокам - вопрос специфичный для конкретного проекта. На многих проектах в начале жизненного пути ещё нет портянок по 100 строк импортов и 1000 строк jsx, сортировать и разделять там почти нечего.

@SQReder
Copy link

SQReder commented Feb 2, 2022

Как ты заметил "в начале проекта". Мы же нацелены на тот, чтобы методология помогала на комплексных проектах, в том числе

В то время как разбивка не мешает на на небольшом количестве импортов, на больших она приносит свою пользу

@azinit
Copy link
Member Author

azinit commented Feb 2, 2022

В то время как разбивка не мешает на на небольшом количестве импортов,

Оч спорно)

Мы же нацелены на тот, чтобы методология помогала на комплексных проектах, в том числе

Но это не значит, что линтер должен халтурно обрабатывать на средних 🤔
Тут кажется небольшая подмена понятий

@azinit
Copy link
Member Author

azinit commented Feb 4, 2022

@tednaaa
Copy link
Member

tednaaa commented Feb 16, 2022

хочется import-order/experimental, но без таких пробелов

image

в целом если большой проект, думаю не составит труда поиском IDE найти нужный импорт, а разделять локальные/внешние модули более комфортно кмк

@Kelin2025
Copy link

Kelin2025 commented Feb 16, 2022

Да просто сделайте дополнительную опцию - с пробелами и без.
Какой-то нелогичный опрос "экспериментальный с пробелами" или "обычный без пробелов" в итоге

@Kelin2025
Copy link

Также мне не нравятся названия "base/experimental". Как будто бы второе - что-то нестабильное, нерекомендованное, притом что объективно выглядит более логичным.
Нужны нейтральные названия, отражающие идею порядка, а не диктующие, что база, а что нет.

@azinit
Copy link
Member Author

azinit commented Feb 16, 2022

Да просто сделайте дополнительную опцию - с пробелами и без.

И на каждую доп.опцию - по доп. конфигу, угу))

Какой-то нелогичный опрос "экспериментальный с пробелами" или "обычный без пробелов" в итоге

Также мне не нравятся названия "base/experimental". Как будто бы второе - что-то нестабильное, нерекомендованное, притом что объективно выглядит более логичным.

Опрос потому и нужен, чтобы зафиксировать что-то одно за "рекомендованное" 🤔
То, о чем написано в описание тикета - увы парочкой строчек не переопределишь (только если полностью "еджектить" конфиг)

Т.е. буквально нужна БАЗА, которая была бы для ~большинства приверженцев FSD - была бы логичной и последовательной
(потому столько фидбека и собираем как нигде)

Как будто бы второе - ... притом что объективно выглядит более логичным.

Ну вот без обид, но это мнение конкретно тебя, а нужно в целом смотреть на картину, люди разные)
Мы вот по большей части сортировали импорты в прямом порядки, и чет ни разу не возникало вопросов "А почему бы не ревертнуть для консистентности" 😏

@azinit
Copy link
Member Author

azinit commented Feb 16, 2022

Причем хочется решить это до условного офиц.релиза линтера, чтобы люди не офигевали от выбора всевозможных разных конфигов и просто юзали готовое
(мы как никак за совместимость)

@azinit
Copy link
Member Author

azinit commented Feb 16, 2022

Мб @Krakazybik еще что скажет

@Kelin2025
Copy link

Kelin2025 commented Feb 16, 2022

И на каждую доп.опцию - по доп. конфигу, угу))

Речь вроде всего о двух.
И здесь есть явно два разных критерия - порядок и отступы. А вы смешали их в один опрос.
Если я хочу базу с отступами или экспериментальный без них - что выбирать?

Опрос потому и нужен, чтобы зафиксировать что-то одно за "рекомендованное"

Какой формат дат рекомендованный - российский или американский?
Этот опрос как раз того же уровня, причём даже идея та же - "DD/MM/YYYY" или "MM/DD/YYYY".

Ну вот без обид, но это мнение конкретно тебя, а нужно в целом смотреть на картину, люди разные)

"Объективно" я написал потому, что оно соблюдает общий порядок. И этот порядок является негласным стандартом во всей разработке (third-partty first -> internal below). Не только во фронте причём.
И называть "базой" конфиг, который противоречит тому, как делает большинство разработчиков, только потому что в доке FSD исторически написали наоборот - это де факто опираться на гораздо меньшую выборку.
И даже если в опросе больше людей проголосовало бы за этот вариант, это всё ещё не было бы пруфом того, что больше людей действительно хотят так - потому что опрос проводится по сути среди узкой аудитории юзеров методологии (т.к. мимопроходилы сюда разве что случайно попадут), что делает его по дефолту нерепрезентативным и бессмысленным.

@Krakazybik
Copy link
Member

Да просто сделайте дополнительную опцию - с пробелами и без.

Если бы была возможность как-то настраивать конфиг через какие-либо аргументы - у нас бы отпало наверное 80% проблем. И это сейчас самая большая боль. К сведению, пока-что данная опция - это х2 к числу конфигов. Если есть сведения, как это сделать, какой-то магией передать туда аргументы, то очень прошу поделиться.

Также мне не нравятся названия "base/experimental". Как будто бы второе - что-то нестабильное, нерекомендованное, притом что объективно выглядит более логичным.
Нужны нейтральные названия, отражающие идею порядка, а не диктующие, что база, а что нет.

На данном этапе - он рили эксперементальный. И был добавлен для ресёрча, опытов.

что больше людей действительно хотят так

Возможно стоит расширить аудиторию опроса, либо переформулировать вопрос с бОльшим количеством вариантов. (где и реакт вверху и тд). Но кмк проголосовавших будет ещё меньше(возможно это наша ошибка, что поставили перед фактом 2х вариантов). Но это не точно =)

В общем:

Кмк, тут как про фломастеры(холивар и только) и такое чувство что кто-то в итоге останется обделённым, как бы мы не старались. По большому счету import-order ни на что не влияет и к методологии имеет вообще косвенное отношение и это про "КРОСИВОЕ". На начальном этапе кажется, если мы перекроем потребности большинства - это лучший вариант(потому данные опросы и проводим). P.S. Есть мысли по конфигурации через некий npx cli, и возможно там уже будет возможность настроить так, как хочется каждому, но это пока только в планах/мыслях.

Пы.Сы.2 Хочется сделать хорошо и приятно всем и никого не обижать, так что будте котиками: если есть отдельное пожелание/своё мнение и оно кажется более правильным, оставьте конкретный пример, чтобы и люди переходящие сюда и мы не агрегировали все комменты, а просто увидев какую-то конкретику про себя сказали "о, я тоже так хочу" и могли как-то пролайкать/дать знать о своих хотелках.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants