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

Does not Sort in One Go #26

Open
FlorianWendelborn opened this issue Mar 18, 2021 · 12 comments
Open

Does not Sort in One Go #26

FlorianWendelborn opened this issue Mar 18, 2021 · 12 comments

Comments

@FlorianWendelborn
Copy link

FlorianWendelborn commented Mar 18, 2021

In very large and nested type definitions, I’ve noticed that this plugin needs up to 5 runs in order to fix all reported sorting issues. First time it finds ~3000, then ~500, then ~50, then 1, then 0.

This wouldn’t be a big deal for manually written code as it rarely changes in significant ways. The file in question is an auto-generated typescript definition file for an entire OpenAPI specification though

@infctr
Copy link
Owner

infctr commented Mar 22, 2021

I'd happily accept a PR addressing the issue

@jamiehaywood
Copy link

@FlorianWendelborn did you start on a PR for fixing this? I'm experiencing the same problem with linting OpenAPI generated type files.

@adityamatt
Copy link

I have the same problem, is anyone working on it ?

@FlorianWendelborn
Copy link
Author

FlorianWendelborn commented Feb 23, 2023

I’m not working on it as I no longer need this particular solution. It’s still a good improvement though unless it’s already fixed so I’m not closing it

@jdk2pq
Copy link

jdk2pq commented May 26, 2023

I had a similar problem with an autogenerated TypeScript file from a GraphQL schema and found a workaround. Running the eslint -- fix command in a bash until loop ended up doing the trick for me. Here's the script in my package.json:

"scripts": {
    "eslint-gql":  "bash -c \"until eslint 'types/api/graphql.ts' --fix --cache; do echo \\\"linting again...\\\"; done\""
}

Not the most elegant solution, but it'll work for now. Hope it helps!

@zachbryant
Copy link

zachbryant commented Jun 9, 2023

I've started working on this. Too much of a pet peeve to introduce to my repo, everyone will hate me if they have to save 5 times 😅

So far I've added a test with 400 ~500 entries, 150 450+ to be sorted. Will be a good baseline to go from

@zachbryant
Copy link

zachbryant commented Jul 4, 2023

I think the reason it doesn't sort in one go is due to conflicting fixes, and the nature of reporting each fix as a swap of nodes (docs on conflicting fixes). ESLint will rerun the plugin 10 times but that's not enough for large files. That, and I noticed some error messages like a should be before A when a should be before _ is more precise. I didn't dig too much further into it before deciding that swapping nodes probably isn't the best approach moving forward.

Given all this, the fix I'm working on includes:

  1. Adding an error to the declaration, where the fixer rewrites the entire body in one go, performing no swaps. As a result:
  2. Changing errors to report their correct placement but not be individually sortable
    • Some errors will say $ should be at the end as a result of this
  3. Modifying test cases to match this
    • Shortening the test case code to be more manageable with even more preprocessing of cases

It's slow going, but the breadth of test cases already included feel like a good safety net 🙂

@adityamatt
Copy link

Hi,
Thanks @zachbryant for looking into it.
Just following up if this is still being fixed?

@zachbryant
Copy link

@adityamatt yep, still planning to create a PR for my changes. The scale is absolutely massive but I'm in the stages of fixing tests to match the new errors

@zachbryant
Copy link

Just got all of the tests passing. I still want to do some things like testing in supported Node environments and fleshing out test cases, but it's finally come together

@adityamatt
Copy link

Happy to hear that!!

@eltonio450
Copy link

hi there :)

what is the current status of this issue :) ? What is the recommended solution ?

Thank you!

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

No branches or pull requests

7 participants