fix: validate-jsdoc-codeblock handles union symbols regardless of order#1345
fix: validate-jsdoc-codeblock handles union symbols regardless of order#1345taiyakihitotsu wants to merge 1 commit intosindresorhus:mainfrom
validate-jsdoc-codeblock handles union symbols regardless of order#1345Conversation
95811ea to
c08db43
Compare
c08db43 to
eeb9889
Compare
|
@som-sm I am not entirely confident in my approach, particularly here: const isNotPassed = (/\/\/}/g.test(actualComment)) || (actualComment.length < 80 && (/\[{\n/g.test(actualComment)));My logic assumes that both To simplify the implementation, I used regular expressions to exclude unsupported patterns. Does this align with your intent, or is there any part of the implementation that seems off? Thanks for reading. |
validate-jsdoc-codeblock accepts union symbolsvalidate-jsdoc-codeblock handles union symbols regardless of order
| namespace AAA { | ||
| export type aaa = {a: string}; | ||
| } | ||
| namespace BBB { | ||
| export type aaa = {a: string}; | ||
| } | ||
| namespace CCC { | ||
| export type aaa = {a: string}; | ||
| export type bbb = {a: string}; | ||
| } | ||
|
|
||
| type Symbols_0 = AAA.aaa | CCC.bbb | CCC.aaa | BBB.aaa; | ||
| //=> AAA.aaa | BBB.aaa | CCC.aaa | CCC.bbb | ||
|
|
||
| type Symbols_1 = AAA.aaa | CCC.bbb | CCC.aaa | BBB.aaa; | ||
| //=> BBB.aaa | AAA.aaa | CCC.aaa | CCC.bbb |
There was a problem hiding this comment.
Umm...didn't get this. This example already works, and the result is already sorted.
namespace AAA {
export type aaa = {a: string};
}
namespace BBB {
export type aaa = {a: string};
}
namespace CCC {
export type aaa = {a: string};
export type bbb = {a: string};
}
type Symbols_0 = AAA.aaa | CCC.bbb | CCC.aaa | BBB.aaa;
// ^? type Symbols_0 = AAA.aaa | BBB.aaa | CCC.aaa | CCC.bbb
type Symbols_1 = AAA.aaa | CCC.bbb | CCC.aaa | BBB.aaa;
// ^? type Symbols_1 = AAA.aaa | BBB.aaa | CCC.aaa | CCC.bbbPlayground: https://tsplay.dev/W4oPAm
There was a problem hiding this comment.
Sorry.
The goal was to verify that both actual and expected are sorted consistently even if they are Union types, allowing them to be compared via actualComment !== expectedComment regardless of their order.
However, I'm concerned that my test case might not be reproducing this scenario correctly.
This is what I want to fix:
https://github.com/sindresorhus/type-fest/actions/runs/21571234433/job/62150630102?pr=1338
If you pull this PR #1338 and run npm test, can you reproduce the same error?
There was a problem hiding this comment.
This is what I want to fix:
https://github.com/sindresorhus/type-fest/actions/runs/21571234433/job/62150630102?pr=1338
Refer #1345 (comment).
|
@taiyakihitotsu This error seems fine: 
If you change something that alters the output of The intent behind custom sorting was to only sort numbers, because unordered numbers can create confusion. For everything else, we want to stick to what shows up in the Quick Info tooltip. Custom sorting everything, will only make things more confusing. So if the Quick Info tooltip shows Also, at times you can tweak the implementation and get the desired order, so it's worth double-checking that as well. |
|
Thank for explanation!
But in my environment, the main branch passes My concern is that if we don't sort Unions, we might end up having to update (unrelated) documentation every time there's a change, just because the output order shifted. (Regardless of whether my approach is the right way to go or not) What do you think? |
Because your PR most likely introduces some change that affects the order of
But the order wouldn't change unless the underlying implementation changes. If the implementation is the same, the order should/would remain the same. And, if you look at the example, the existing order is correct, because it follows the order in the input object.
|
Thank you for clarifying! I withdraw this PR (and reopen #1338). Thanks! |
In the current (before this PR)
validate-jsdoc-codeblocks.jsdoesn't sort union types including symbols.And TypeScript doesn't ensure the order or unions, whether it's placed in
actualCommentorexpectedComment.This PR sorts both, keeps the original comments to pass a report.
Background
The union order error doesn't occur in the current main, 051325a including the commit to fix path, 8f0419c, but in my PR: #1338
Error log
PS
Changed this PR #1338 to draft because of this PR.