feat: add import and export rules by susnux · Pull Request #981 · nextcloud-libraries/eslint-config

susnux · 2025-04-10T09:57:09Z

faster alternative for feat(imports): add ESLint imports plugin #973

Enforce file extensions
Enforce consistent named import / export sorting
Enforce consistent module sorting

Do not use eslint-plugin-import(-x) because it is quite slow and has many issues with parsing different formats like vue script-setup in Typescript and similar.

lib/configs/imports.ts

ShGKme · 2025-04-10T10:39:30Z

import type { NcSelectOption } from '../composables/useNcSelectModel.ts'                        // type

import { t } from '@nextcloud/l10n'                                                             // external (@nextcloud)
import NcButton from '@nextcloud/vue/components/NcButton'                                       // external (Vue) (@nextcloud)
import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch'             // external (Vue) (@nextcloud)
import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'                                   // external (Vue) (@nextcloud)
import NcTextField from '@nextcloud/vue/components/NcTextField'                                 // external (Vue) (@nextcloud)
import { storeToRefs } from 'pinia'                                                             // external
import { computed, ref } from 'vue'                                                             // external
import { ZOOM_MAX, ZOOM_MIN } from '../../../constants.js'                                      // INTERNAL
import { useNcSelectModel } from '../composables/useNcSelectModel.ts'                           // INTERNAL
import { useAppConfigStore } from './appConfig.store.ts'                                        // INTERNAL
import { useAppConfigValue } from './useAppConfigValue.ts'                                      // INTERNAL
import SettingsFormGroup from './components/SettingsFormGroup.vue'                              // INTERNAL (Vue)
import SettingsSelect from './components/SettingsSelect.vue'                                    // INTERNAL (Vue)
import SettingsSubsection from './components/SettingsSubsection.vue'                            // INTERNAL (Vue)
import IconBellRingOutline from 'vue-material-design-icons/BellRingOutline.vue'                 // external (Vue)
import IconCardAccountPhoneOutline from 'vue-material-design-icons/CardAccountPhoneOutline.vue' // external (Vue)
import IconMagnify from 'vue-material-design-icons/Magnify.vue'                                 // external (Vue)
import IconMinus from 'vue-material-design-icons/Minus.vue'                                     // external (Vue)
import IconPhoneRingOutline from 'vue-material-design-icons/PhoneRingOutline.vue'               // external (Vue)
import IconPlus from 'vue-material-design-icons/Plus.vue'                                       // external (Vue)
import IconRestore from 'vue-material-design-icons/Restore.vue'                                 // external (Vue)
import IconThemeLightDark from 'vue-material-design-icons/ThemeLightDark.vue'                   // external (Vue)
import IconVolumeHigh from 'vue-material-design-icons/VolumeHigh.vue'                           // external (Vue)

IMHO:

External modules vue-material-design-icons should not go after internal even they are .vue
Would be nice to have imports like from 'vue' on top of from '@nextcloud/*'
Would be nice to have @nextcloud/vue components close to all the other components
I'd also put styles to the end to side effects

import type { NcSelectOption } from '../composables/useNcSelectModel.ts'                        // type

import { storeToRefs } from 'pinia'                                                             // external
import { computed, ref } from 'vue'                                                             // external
import { t } from '@nextcloud/l10n'                                                             // external (@nextcloud)
import NcButton from '@nextcloud/vue/components/NcButton'                                       // external (Vue) (@nextcloud)
import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch'             // external (Vue) (@nextcloud)
import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'                                   // external (Vue) (@nextcloud)
import NcTextField from '@nextcloud/vue/components/NcTextField'                                 // external (Vue) (@nextcloud)
import IconBellRingOutline from 'vue-material-design-icons/BellRingOutline.vue'                 // external (Vue)
import IconCardAccountPhoneOutline from 'vue-material-design-icons/CardAccountPhoneOutline.vue' // external (Vue)
import IconMagnify from 'vue-material-design-icons/Magnify.vue'                                 // external (Vue)
import IconMinus from 'vue-material-design-icons/Minus.vue'                                     // external (Vue)
import IconPhoneRingOutline from 'vue-material-design-icons/PhoneRingOutline.vue'               // external (Vue)
import IconPlus from 'vue-material-design-icons/Plus.vue'                                       // external (Vue)
import IconRestore from 'vue-material-design-icons/Restore.vue'                                 // external (Vue)
import IconThemeLightDark from 'vue-material-design-icons/ThemeLightDark.vue'                   // external (Vue)
import IconVolumeHigh from 'vue-material-design-icons/VolumeHigh.vue'                           // external (Vue)
import SettingsFormGroup from './components/SettingsFormGroup.vue'                              // INTERNAL (Vue)
import SettingsSelect from './components/SettingsSelect.vue'                                    // INTERNAL (Vue)
import SettingsSubsection from './components/SettingsSubsection.vue'                            // INTERNAL (Vue)
import { ZOOM_MAX, ZOOM_MIN } from '../../../constants.js'                                      // INTERNAL
import { useNcSelectModel } from '../composables/useNcSelectModel.ts'                           // INTERNAL
import { useAppConfigStore } from './appConfig.store.ts'                                        // INTERNAL
import { useAppConfigValue } from './useAppConfigValue.ts'                                      // INTERNAL

lib/configs/imports.ts

Antreesy · 2025-04-10T13:34:48Z

lib/configs/imports.ts

+							// everything else which is everything internal
+							'unknown',
+							// import style from 'my.module.css'
+							'style',
+							// Vue components
+							'vue',


Agree with Grigorii (but also because I used to this order):

Suggested change

// everything else which is everything internal

'unknown',

// import style from 'my.module.css'

'style',

// Vue components

'vue',

// Imports from @nextcloud ecosystem

'nextcloud',

// Imports from @nextcloud/vue library

'nextcloud-vue',

// Vue components

'vue',

// everything else which is everything internal

'unknown',

// import style from 'my.module.css'

'style',

Antreesy · 2025-04-10T13:35:09Z

lib/configs/imports.ts

+						],
+						customGroups: {
+							value: {
+								vue: '\\.vue$',


Suggested change

vue: '\\.vue$',

nextcloud: '^@nextcloud.*',

'nextcloud-vue': '^@nextcloud\/vue.*',

vue: '\\.vue$',

This what is used in Talk:

// group @nextcloud imports pattern: '@nextcloud/{!(vue),!(vue)/**}', // group @nextcloud/vue imports pattern: '{@nextcloud/vue,@nextcloud/vue/**}',

susnux · 2025-04-20T12:47:06Z

External modules vue-material-design-icons should not go after internal even they are .vue

Fixed ✅

Would be nice to have @nextcloud/vue components close to all the other components

Fixed ✅

I'd also put styles to the end to side effects

Fixed ✅

Would be nice to have imports like from 'vue' on top of from '@nextcloud/*'

What is the use case? Its just a namespace, like every other, I would consider it really weird to see:

import { foo } from '@foo/bar'
import { computed } from 'vue'
import { t } from '@nextcloud/l10n'

instead of consistent alphabetical ordering of normal modules¹:

import { foo } from '@foo/bar'
import { t } from '@nextcloud/l10n'
import { computed } from 'vue'

¹ I mean like type imports or Vue components are a special kind of modules one is just type information that will be stripped away and the other is UI components for use in the template, compared to everything else which is logic code that will be used.

If I import that code form one namespace or another makes no difference for reading the code, it does not provide any information except that is a Nextcloud maintained library but what does that help you in your flow?

TL;DR: I am fine with it if others agree with that sorting but I would prefer only sort by module type rather than package maintainer.

lib/configs/codeStyle.ts

lib/configs/imports.ts

susnux · 2025-04-24T14:09:37Z

Fixed performance issue by using the new config format.

ShGKme

Works well now

ShGKme · 2025-04-24T15:44:00Z

lib/configs/imports.ts

+							// type first
+							'external-type',
+							'type',
+							{ newlinesBetween: 'always' },


Maybe remove these empty lines as well?

So imports are a single block, separated by an empty line from the remaining module code.
Type imports are separated visually via import type prefix anyway.

Suggested change

{ newlinesBetween: 'always' },

{ newlinesBetween: 'ignore' },

Maybe?

Antreesy

Gonna be painful to get used to it

Antreesy · 2025-04-24T17:24:58Z

lib/configs/imports.ts

+							// everything else which is everything internal
+							'unknown',
+							// Vue components
+							'vue', // external modules (e.g. nextcloud-vue)


So we decided to go with this? external .vue components are now listed after internal non-vue modules

@Antreesy one problem here is that you use the old imports they are not recognized as vue components. (The /dist/Components).
Instead use /components/Nc...

From the config it is:

external

internal

vue external

vue internal

Which is indeed different from the proposal above

It was always like that here 👀

Typescript (external | internal)

Javascript (external | internal)

Vue (external | internal)

Styles (external | internal)

Otherwise why should you sort vue independently if you mix it with Javascript?

Typescript (external | internal)
Javascript (external | internal)

Does it actually separate TS from JS, or types from modules?

Otherwise why should you sort vue independently if you mix it with Javascript?

To not break it in the middle, because it can be in a single batch without mixing externals with internals

Does it actually separate TS from JS, or types from modules?

I do not mean .ts files I meat types.

Types are stripped away so that block is only for type checking.

Javascript is the code that stays so its the most important block.

Vue files are components, you do not care in <script> about them so they do not need to interrupt reading the script sources. They are only relevant for the <template>.

- Enforce file extensions - Enforce consistent named import / export sorting - Enforce consistent module sorting Do not use eslint-plugin-import(-x) because it is quite slow and has many issues with parsing different formats like vue script-setup in Typescript and similar. Signed-off-by: Ferdinand Thiessen <[email protected]>

Signed-off-by: Ferdinand Thiessen <[email protected]>

ShGKme · 2025-04-24T23:03:56Z

External modules vue-material-design-icons should not go after internal even they are .vue

Fixed ✅

This is why I thought, it was fixed, and externals don't go after internals

susnux · 2025-04-24T23:47:30Z

Well then it was a misunderstanding. Currently there is really mixed sorting in e.g. server, but for me it seemed to be agreed that Vue components are sorted extra as they belong to <template> rather than <script>.
Thats where this sorting was coming from.

In most components this leads to

import { computed } from 'vue'
import { getSomething } from '../../utils/myUtil.ts'
import NcButton from '@nextcloud/vue/components/NcButton'
import MyComponent from './MyComponent.vue'

Which I thought is more common than

import { computed } from 'vue'
import NcButton from '@nextcloud/vue/components/NcButton'
import MyComponent from './MyComponent.vue'
import { getSomething } from '../../utils/myUtil.ts'

So it was about seeing directly what are components vs seeing what origins from where.
But I do not mind a different sorting if it is agreed upon at least its consistent.

ShGKme · 2025-04-25T09:16:52Z

I'd continue the discussion in chore: Revert "fix(imports): do not mix externals and internals" #1002 to have it in one place

susnux added enhancement New feature or request 3. to review labels Apr 10, 2025

susnux force-pushed the feat/imports-fast branch 2 times, most recently from 40be8f7 to 0f2d689 Compare April 10, 2025 10:00

This comment was marked as resolved.

Sign in to view

ShGKme reviewed Apr 10, 2025

View reviewed changes

lib/configs/imports.ts Outdated Show resolved Hide resolved

Antreesy reviewed Apr 10, 2025

View reviewed changes

This was referenced Apr 11, 2025

feat(imports): add ESLint imports plugin #973

Closed

feat(imports): sort imports in a consistent way #976

Closed

susnux force-pushed the feat/imports-fast branch from e31110e to c3c8f1c Compare April 20, 2025 12:27

ShGKme reviewed Apr 21, 2025

View reviewed changes

lib/configs/codeStyle.ts Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

ShGKme reviewed Apr 24, 2025

View reviewed changes

lib/configs/imports.ts Outdated Show resolved Hide resolved

susnux force-pushed the feat/imports-fast branch from c3c8f1c to d0623b1 Compare April 24, 2025 14:09

This comment was marked as resolved.

Sign in to view

susnux requested a review from ShGKme April 24, 2025 14:21

susnux force-pushed the feat/imports-fast branch from 4b33d4d to a523673 Compare April 24, 2025 14:27

ShGKme approved these changes Apr 24, 2025

View reviewed changes

Antreesy approved these changes Apr 24, 2025

View reviewed changes

susnux added 2 commits April 24, 2025 23:17

chore: adjust internal code to new rules

aca2d69

Signed-off-by: Ferdinand Thiessen <[email protected]>

susnux force-pushed the feat/imports-fast branch from a523673 to aca2d69 Compare April 24, 2025 21:17

susnux merged commit cc3d228 into main Apr 24, 2025
10 checks passed

susnux deleted the feat/imports-fast branch April 24, 2025 21:18

susnux mentioned this pull request Apr 24, 2025

[question] Import style #999

Closed

ShGKme mentioned this pull request Apr 24, 2025

fix(imports): do not mix externals and internals #1000

Merged

	{ newlinesBetween: 'always' },
	{ newlinesBetween: 'ignore' },

Comments

Conversation

susnux commented Apr 10, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

ShGKme commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Antreesy Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Antreesy Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

susnux commented Apr 20, 2025

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

susnux commented Apr 24, 2025

Uh oh!

This comment was marked as resolved.

ShGKme left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Antreesy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

susnux Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShGKme Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShGKme commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

susnux commented Apr 24, 2025

Uh oh!

ShGKme commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ShGKme commented Apr 10, 2025 •

edited

Loading

Antreesy Apr 10, 2025 •

edited

Loading

Antreesy Apr 10, 2025 •

edited

Loading

susnux Apr 24, 2025 •

edited

Loading

ShGKme Apr 24, 2025 •

edited

Loading

ShGKme commented Apr 24, 2025 •

edited

Loading

ShGKme commented Apr 25, 2025 •

edited

Loading