Repository navigation
fix: improve-scripts-sorting - #376
Conversation
| 'test:a': 'foo', | ||
| 'test:b': 'bar', |
There was a problem hiding this comment.
Can we also test scripts contains mutiple :s? Eg: test:a:a, test:a:b
There was a problem hiding this comment.
Sure,
I have now added the test for this scenario.
| 'test:a:a': 'foofoo', | ||
| 'test:a:b': 'foobar', |
There was a problem hiding this comment.
| 'test:a:a': 'foofoo', | |
| 'test:a:b': 'foobar', | |
| 'test:a:a': 'foofoo', | |
| 'test:a:b': 'foobar', | |
| 'test:a-coverage': 'foobar', |
There was a problem hiding this comment.
Is the expected sorted file like this
'test:a',
'test:a-coverage',
'test:a:a',
'test:a:b',or this
'test:a',
'test:a:a',
'test:a:b',
'test:a-coverage',There was a problem hiding this comment.
If we want the sorting to be like this
'test:a',
'test:a:a',
'test:a:b',
'test:a-coverage',we can replace the entire logic with
source = keys.slice().sort((a, b) =>
a.replace(/:/g, ' ')
.localeCompare(b.replace(/:/g, ' '), 'en'));else the current code will sort it in this way
'test:a',
'test:a-coverage',
'test:a:a',
'test:a:b',
Co-authored-by: fisker Cheung <lionkay@gmail.com>
| } | ||
| source = Array.from(groupMap.keys()) | ||
| .sort() | ||
| .flatMap((groupKey) => groupMap.get(groupKey).sort()) |
There was a problem hiding this comment.
I think this should do recursively
There was a problem hiding this comment.
I don't think I am following. Could you tell me why?
There was a problem hiding this comment.
The "group" can be deep
test
test:a
test:b
test-coverage
test:production
test:production:a
test:production:b
test:production-coverage
test:production:cjs
test:production:cjs:a
test:production:cjs:b
test:production:cjs-coverage
test:production:mjs
test:production:mjs:a
test:production:mjs:b
test:production:mjs-coverage
There was a problem hiding this comment.
Also, current implementation seems relay on - and : order in ascii, it shouldn't
test
test:a
test:b
test[NO MATTER WHAT IS] # this should still put after the ones with `:`There was a problem hiding this comment.
Hi @fisker , I was not able to group them as above. But after sorting it will be like
'test', <--------- parent
'test:a', <--------- child
'test:ab',
'test:b',
'test:production',
'test:production:a',
'test:production:b',
'test:production:cjs',
'test:production:cjs:a',
'test:production:cjs:b',
'test:production:cjs-coverage',
'test:production:mjs',
'test:production:mjs:a',
'test:production:mjs:b',
'test:production:mjs-coverage',
'test:production-coverage',
'test-coverage' <--------- overall coverageWill this work?
Also the below three tests are flaky for me. Is this related to my changes?
✘ [fail]: cli › run `cli` on 2 good files and 2 bad files Should format 2 files.
✘ [fail]: cli › run `cli --check` on 2 bad files Should report 2 files.
✘ [fail]: cli › run `cli` on 2 bad files Should format 2 files.
fisker
left a comment
fisker
left a comment
There was a problem hiding this comment.
The result looks great! Thank you!
|
🎉 This PR is included in version 3.5.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closes #360
Grouping keys before : and sorting the keys and the items in the group of the respective keys and then flattening the array.