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: ability to sort type only named imports and exports first or last #664

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Sep 17, 2024

Two new options:

  • importDeclaration.sortTypeOnlyImports
  • exportDeclaration.sortTypeOnlyExports

With possible values:

  • first - Sorts type only named imports/exports first
  • last (default) - Sorts type only named imports/exports last
  • none - Does not sort based on if a type only named import or export

For denoland/deno#22583

@dsherret dsherret merged commit 6b402b3 into main Sep 17, 2024
3 checks passed
@dsherret dsherret deleted the feat_sort_type_only_named_imports_exports branch September 17, 2024 15:20
@jakebailey
Copy link
Contributor

jakebailey commented Sep 17, 2024

The old behavior was none, but the new default is last; was that intended?

FWIW TS supports multiple sort orderings here (since 5.4 microsoft/TypeScript#55269), but tries to detect what ordering is in use to avoid modifying it, along with some (yet to be documented) options to control the behavior.

(also lydell/eslint-plugin-simple-import-sort#124 where I unfortunately advocated for the inline/none behavior dprint had to attempt to align all of the tools...)

@dsherret
Copy link
Member Author

Yeah I changed it to align with the upcoming deno behaviour. Maybe it should be kept as-is? I'd argued internally to keep it as-is to minimize diffs, but it was pointed out that most things will rarely flip back and forth (as well as the ts organize imports behaviour, which doesn't seem to be the case anymore as you pointed out). Thoughts?

@dsherret
Copy link
Member Author

Ref: #666 (comment)

@jakebailey
Copy link
Contributor

Personally, I much prefer the inline / none behavior because it prevents complete resorting of entries when they flip between type and non type imports. For compat, TS made the choice to put them at the end by default if there is no other info or config (since people sort of depend on our existing behaviors), but our intent was to eventually expose these configs (they're present as experimental ones at least), and potentially have "presets" to let people opt into the more common behavior, which before this PR was acutally inline... 😄

@dsherret
Copy link
Member Author

Thanks! I reverted it and published a new version.

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