Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

v2.23 broke alphabetical sorting #2064

Closed
jeron-diovis opened this issue May 14, 2021 · 6 comments · Fixed by #2071
Closed

v2.23 broke alphabetical sorting #2064

jeron-diovis opened this issue May 14, 2021 · 6 comments · Fixed by #2071

Comments

@jeron-diovis
Copy link

jeron-diovis commented May 14, 2021

This and this changes cause the following weird effect.

With imports like this:

import React from 'react'
import { BrowserRouter } from 'react-router-dom'
...

func mutateRanksToAlphabetize produces following groups:

{
  '0.8': [ 'react-value', 'react-router-dom-value' ],
  ...
}

Then this group gets sorted alphabetically, and obviously turns into [ 'react-router-dom-value', 'react-value' ]. While normally, 'react' < 'react-router-dom' === true.

So after updating to 2.23 I suddenly have warnings about wrong imports order all over the project .

Probably, you may just use | instead of - to separate importKind from importName?

@ruscon

This comment has been minimized.

@ljharb
Copy link
Member

ljharb commented May 14, 2021

cc @grit96 / #2021 - could you take a look at this?

@heath-freenome

This comment has been minimized.

@geraintwhite
Copy link
Contributor

Perhaps the change should be reverted for now. I will try to take a look at fixing it next week.

@ljharb
Copy link
Member

ljharb commented May 14, 2021

That's a pretty major thing to revert, I'm hoping we can roll forward.

@geraintwhite
Copy link
Contributor

I've created a fix in #2071 and added a test for the code in the OP

@ljharb ljharb closed this as completed in 83f3c3e May 14, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging a pull request may close this issue.

5 participants