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

[Docs] extensions, order: improve documentation #3106

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

Xunnamius
Copy link
Contributor

Resolves #2897
Related to #3104 and #3105
Potentially related to #2708, #2304, #2540

I can slice and dice this PR in whatever way is most useful.

The changes to the import/extensions documentation (preview) are not so massive and should merge cleanly.

The changes to the import/order documentation (preview), however, are much more extensive. I've been referring back to these docs on and off for several months as I've tweaked my linter configurations, and every time I do I pretty much leave confused and fall back on stepping through eslint-plugin-import's source to see what's actually happening. Along with documenting the new functionality introduced in #3104, this PR attempts to demystify import/order's documentation in the following ways:

  • There is no documentation describing how an import is grouped. What classifies an import as "external" vs "internal" vs "builtin"? What does "sibling" or "parent" actually mean? Every time I come back to these docs, I find myself looking for a technical description that doesn't exist and so I ultimately guess, which leads to problems 😅. For this PR, I've looked through the rule's source and have attempted to describe the grouping algorithm with specificity. Since I've only spent a day or so with this repo's source code, it may not be totally accurate and should be scrutinized for mistakes.

  • Some of the examples don't work. They've been fixed (and feat(import/order): enable advanced spacing and sorting of type-only imports #3104 adds corresponding tests for them).

  • The options headings contain their type description alongside their names. This means references into this document from the wider web will break whenever one of the settings is tweaked. I've encountered several of these "dead links" over time. Worse, they make for truly ugly headings. I believe it best to instead use short simple headings that are unlikely to change, such as each option's full name alone.

  • The type descriptions of the options (e.g. [array of this or that]) and their defaults are vague and confusing and are documented inconsistently. Some of the descriptions of the functionality of each option are similarly confusing, such as the description for pathGroups. In my review of the issues literature, I found I was not alone in this. This is my stab at making this aspect of the documentation more approachable.

  • There are several "gotchas" and "footguns" when using some of these options. In this PR, these are now shouted out using GFM alert syntax.

  • Option sections are formatted inconsistently. Several of the examples and their descriptions are formatted differently than the others. This PR ensures all sections are using consistent formatting and structure.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.17%. Comparing base (a20d843) to head (31b786b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3106      +/-   ##
==========================================
- Coverage   95.28%   95.17%   -0.12%     
==========================================
  Files          83       83              
  Lines        3584     3583       -1     
  Branches     1252     1251       -1     
==========================================
- Hits         3415     3410       -5     
- Misses        169      173       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ljharb ljharb force-pushed the contrib-documentation-updates branch from 31b786b to 44827e7 Compare November 27, 2024 03:09
@ljharb
Copy link
Member

ljharb commented Nov 27, 2024

@Xunnamius just curious; did you use an LLM in any part of your PR description?

@ljharb ljharb added the docs label Nov 27, 2024
@ljharb ljharb changed the title docs: update documentation for import/order and import/extensions [Docs] extensions, order: fix a couple typos Nov 27, 2024
@ljharb ljharb changed the title [Docs] extensions, order: fix a couple typos [Docs] extensions, order: improve documentation Nov 27, 2024
@ljharb ljharb force-pushed the contrib-documentation-updates branch 2 times, most recently from 17f2f5b to d5f2950 Compare November 27, 2024 03:38
@Xunnamius
Copy link
Contributor Author

Xunnamius commented Nov 27, 2024

@Xunnamius just curious; did you use an LLM in any part of your PR description?

@ljharb Haha, no 🤭. I write the docs for the fancy linear algebra, it doesn't madlib my docs for me. I hope my writing doesn't read like slop!

And thanks for taking a look at the PR 👍

@ljharb
Copy link
Member

ljharb commented Nov 27, 2024

@Xunnamius glad to hear it, it'd have been the first time it wasn't immediately obvious to me :-p your writing is fine :-)

@ljharb ljharb merged commit d5f2950 into import-js:main Nov 27, 2024
339 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

import/order pathGroups and pathGroupsExcludedImportTypes is too confusing
2 participants