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

feat: allows users to enter { newlinesBetween } in groups #435

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

hugop95
Copy link
Contributor

@hugop95 hugop95 commented Dec 26, 2024

Resolves #401
Continuation of #428.

  • PR 1: Add customGroup.newlinesInside option. We will take the opportunity to create a getNewlinesBetweenOption method that will help us later to precisely customize newline behavior between two specific groups.
  • ⚙️ PR 2 (this): Add a way to choose a specific newline behavior between two groups. See the proposals here.
    • Proposal 2 is implemented here.

Description

Allows users to enter { newlinesBetween: 'always' | 'never' | 'ignore' } between elements of the groups option. This enables precisely setting/overriding the newline behavior between two groups.

Example:

{
  newlinesBetween: "always", // Default behavior
  groups: [
    "public-property",
    { newlinesBetween: "never" }, // Will override the default behavior
    "protected-property",
    { newlinesBetween: "never" },
    "private-property",
    // Will enforce a newline here because of the default behavior
    "method",
  ]
}
  • { newlinesBetween: 'ignore' } is only useful if a newlinesBetween option different from ignore is set.
  • Entering two consecutive { newlinesBetween } objects in a group configuration will raise an error: Consecutive 'newlinesBetween' objects are not allowed.

Impacted rules

  • sort-imports
  • sort-intersection-types
  • sort-union-types
  • sort-classes
  • sort-modules
  • sort-object-types

Additional notes

❓ Is this better than proposal 1 ?

Proposal 1:

Adds customGroups.newlinesAbove and customGroups.newlinesUnder options

You can see a proof of concept PR here.

I believe the current implementation is superior:

  • Implementing proposal 1 means handling cases where the below group has a newlinesAbove incompatible with the newlinesUnder of the above group.
  • It's not as easy to read the newline configuration. This implementation allows users to quickly see where newlines are enforced (or not) by looking at groups.
  • Proposal 1 can co-exist with proposal 2, so it's always theoretically possible to have both implementations later if really needed.

❓ Why not use string tokens instead of an object, such as "newlinesBetween:always" ?

I believe that an object gives us more flexibility in the future if we want to allow users to pass additional options.
It's also easy to distinguish those objects from the group names.

What is the purpose of this pull request?

  • New Feature

@hugop95 hugop95 force-pushed the feat/newlines-between-groups branch from 0cfa6ab to b720a00 Compare December 26, 2024 00:12
@hugop95 hugop95 changed the title refactor: renames props interface feat: allows users to override newlinesBetween in groups Dec 26, 2024
@hugop95 hugop95 changed the title feat: allows users to override newlinesBetween in groups feat: allows users to enter { newlinesBetween } in groups Dec 26, 2024
@hugop95 hugop95 force-pushed the feat/newlines-between-groups branch from b720a00 to 322d514 Compare December 26, 2024 15:59
@hugop95 hugop95 force-pushed the feat/newlines-between-groups branch from 322d514 to d561f91 Compare December 26, 2024 16:19
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.

Feature: sort-imports - add option to sort within sub groups or remove new line between some groups
1 participant