fix: move types condition to the front where needed

[Andarist]

I moved types condition to the front. package.json#exports are order-sensitive - they are always matched from the top to the bottom. When a match is found then it should be used and no further matching should occur.

Right now, the current setup works in TypeScript but it's considered a bug and it should not be relied upon, see the thread and the comment here. For that reason, I would like to fix all popular packages that misconfigured their exports this way so the bug can be fixed in TypeScript.

[shadowspawn]

From https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

The "types" condition should always come first in "exports".

[shadowspawn]

LGTM

[Andarist]

Since this PR is approved - is there a chance that it could land? Or do you have some specific time at which you land approved PRs?

[shadowspawn]

Personally I prefer to keep the main branch in line with the published release, and don't want to release too often.

Realistically, there isn't a rush to land this. It isn't causing known problems and is on a little used entry point.

(In the meantime, you could update the PR to current main, but that is not a blocker.)

[shadowspawn]

The checking website mentioned in the linked thread actually complains about almost everything for yargs! But I assume some of the big X are because most of the end-user types are external via Definitely Typed?

I think I need to do some rereading about how types fits into the conditional exports. All I remember is it is way more complicated than I expected...

https://arethetypeswrong.github.io/?p=yargs

[Andarist]

So this PR's focus is on fixing the "🐛 Used fallback condition" - since that's related to a bug in TypeScript itself. Everything else is out of the scope of this PR. However, I can advise how to properly fix just any of those problems - or even do the work myself if needed (but for that I would have to know more about the project itself).

But I assume some of the big X are because most of the end-user types are external via Definitely Typed?

If your types mainly live in DefinitelyTyped then ye - I would expect that "❌ No types" is OK here since arethetypeswrong likely focuses on the tested package itself (without querying DefinitelyTyped). If that's the case - having some types locally (for the /browser entry) is rather an unusual setup ;p

I think I need to do some rereading about how types fits into the conditional exports. All I remember is it is way more complicated than I expected...

Yes, it's quite complicated - especially if you are trying to ship a "dual package" (and you do). If you have 2 separate implementation files (for require and import conditions) then it's incorrect to assume that you can only use a dingle set of declaration files (even though that's what most assume). The runtime code and the types should match with each other 1 to 1 - if you have 2 different runtime codes you also need 2 separate declaration files (if you want to have 100% correct setup etc).

[shadowspawn]

Just checking my understanding. I think another way of handling this would be to delete the "types" entry? Because the filenames and location of the browser entry point and the definition file match, TypeScript will find the types itself.

(Not requesting a PR change. I am happy listing it.)

[Andarist]

Just checking my understanding. I think another way of handling this would be to delete the "types" entry? Because the filenames and location of the browser entry point and the definition file match, TypeScript will find the types itself.

In this specific situation, it wouldn't be a good change. You can rely on what I call "a sibling lookup" but your extensions should line up. So the problem, here, is that your implementation file is .mjs but the types are .d.ts. This really should be a .mjs/.d.mts or .js/.d.ts pair.

That's likely a problem that will be caught by arethetypeswrong as well - it's just a different problem and this PR focuses on fixing the "🐛 Used fallback condition".

I totally get that this whole thing is quite confusing and not that well-documented. It's rather easy to end up with some subtle problems. If you want to get this completely right, it might be best to schedule a call or something to guide you through this. yargs is a mature project that we don't want to break anyhow, it has a lot of baggage (from the lack of a better word) and not everything here is "conventional". For that reason, I think it's best to just pair (if you want that) - since you are an expert on yargs's setup and all and that's something that I can't just catch up with quickly.

[shadowspawn]

Thanks for the comments.

I missed the inconsistent file extension, good point! I thought that might be ok because of the "type": "module", in package.json, but I tried uploading to arethetypeswrong and did get a big fat red X with the strict checking there.

Keep it simple, just reorder for now...

[Andarist]

I thought that might be ok because of the "type": "module", in package.json

Oh yea, I missed that! In this case - yeee, it should be somewhat OK but it becomes harder to reason about and stuff. So my recommendation is to always keep matching extensions.