update package.json to follow docs + disable sourceMap in prod

[o-az]

Thank you for this amazing library that I use quite frequently. I've made slight updates here and there mainly to config stuff.

Updates made:

• moved "types" inside "exports" to the top of "." entry as suggested in tyoescriptlang.org,
• disabled generating sourceMaps unless environment variable DEBUG is set to true since source maps are for dev/debugging purposes only,
• removed src from dist,
• in devDependencies: added @types/node, updated vitest, and removed tslib,
• in ci.yml updated branches: main to array format cause it was showing an error and github syntax says it should be array,
• prettier/eslint: consolidated prettier rules to one place in package.json and required the object in eslint + enabled cache for eslint,
• enabled type checking in ./scripts and added explicit @ts-ignore for fields that are not part of the imported packages,
• updated tsconfig.json.

Cheers

[kitten]

• exports.type move: I'm aware that some linters and some sources state that this is preferably, at least for TS to be listed first. I haven't done this in any lib I maintain however because I don't see any technical reason for this. Of course, these entries are tested and resolved in order, as if they were a list. However TS only reads this one entry (if at all, since the root entry for types is also valid) and hence doesn't need this to come first. What's most important here is what bundlers and runtimes read, and if they insisted on their option coming first, it's clear that putting this option first is bogus. At least, I haven't seen a technical reason for why it should come first
  • Edit: I also now looked at the doc you linked on this topic, and it doesn't mention that putting it first is recommended, but even required 😅 I did see this pop up in a linter somewhere, but I've literally never seen a mention of why. And I don't see a technical reason of why that's relevant.
• Disabled sourcemaps: Sourcemaps are intentionally built and included. They're not something that should ever not be included, if possible. They must be enabled for every bundle so that consumers of the library can actually compare the code against the source rather than the built output (and the source maps are hence published, see: https://unpkg.com/browse/wonka@6.2.3/dist/). They're hence purposefully enabled in all parts of the Rollup pipeline, output into one map per output, and provided since consumers are also debugging Wonka code when it's involved in an error
• Removed exports.source: I placed this there on purpose and it's included in several libraries I maintain. Again, package.json:exports is an absolute mess of a spec, if it can even be called that. But early on, several maintainers decided to include it, and these fields generally aren't specced anyway.
• Added @types/node: Generally, I don't think it's ever used, unless we use it for scripts (see later note)
• Workflow branches: main: It's a shortcut, and it should work fine. I'll have to check that but I'm fairly certain this is commonly used, and as far as I remember even occurs in the GH docs. I'm not sure if it's deprecated, but it does still work as intended (as many array/singleton swaps do in many yaml configs)
• Prettier/ESLint: this is a change I don't really mind, but I'd love to check whether the ESlint plugins config can in this case then be omitted entirely. That said, since both ESLint and Prettier make changes to the syntax, potentially, this is really only relevant for editors... in some cases, so really only when someone writing code in this repo uses formatting on save... so 😅 I'm not sure if it adds that much value
• Include scripts in type checking: I really wouldn't want to do this without reason tbh. These scripts are literally just scripts; they either work or they don't. Type checks or not. Formatting them and enabling checks now (which also won't really do much for us without actually checking and adding typings) is a little late and will just add changes when they aren't immediately needed
• Updated tsconfig: none of these changes are needed, again, unless we look at scripts. module: 'esnext' in particular however has some real changes in outputs afaik, and was hence split off. I haven't read up on them yet and since this is potentially wide ranging, I'd rather also not change it unless it's needed

Sooooo, I'm sorry for the long text 😅♥️ Basically, I'm glad you're enthusiastic and took the time to submit this. However, making changes is only a part of the story, and mixing them all together, submitting a lot of different tooling/workspace changes, and having so many in places that can have subtle consequences puts a lot of reviewing burden on me

In general, I'm pretty careful when it comes to this in Open Source, and hence always prefer discussions or issues prior. In the current state, this basically now becomes a lot of work to review and discuss altogether, and most changes here aren't needed, i.e. they don't add any direct features, values, don't address issues, and don't fix bugs.

So while I'm open to discussing some of them in isolation, or consider some, there's a more pragmatic point to discuss: How many people realistically would profit from these changes, regardless of their benefit? We have two maintainers of the repo, and very infrequent changes, feature additions, and little movement 😅 Hence, I'm careful on accepting large changes to a "slow" moving (i.e. stable) library, unless they're needed for a feature/change ❤️‍🩹

[kitten]

Ok, just quickly popping another separate comment in here, on what changes I'd immediately accept;

• package.json:exports.type: If I have a technical reason of why it's needed in the future, or proof that no behaviour changes there's no reason not to do it. Although, if no behaviour changes, there's also not really much to say on why it should be made 😅 either way, I'm ambivalent towards it
• ci.yml fix: I checked and this could be changed, after all, it's typed in the non-shortcut version in the other workflow
• Prettier/ESLint: I'd rather look at this myself, unless there's good info on no config working as well. Overall however, I'm open to it. That said though, the precommit won't change and ESlint will have to be the primary formatter, so may not have a big effect

Re. dependencies: I'd explicitly would want to update this, as a maintainer. That's more of a precaution and actually simply saves time, since I wouldn't have to validate what's changed, if I do this myself 😅 That's only a precaution though since in this instance it saves time

Re. typed scripts: I'd like to skip this one. I don't think it'll add a lot of value in these scripts and just increases maintenance burden without the types actually helping us much.

Re. npmrc/package.json:scripts changes: They don't seem needed and it's more of a personal change, so I wouldn't want to apply those

Edit: Also, again, I apologise for this being long ♥️ I do want to find a way to accept some of this, since you spent time on it. But do note that it's unusual to go into OSS repo and make unrequested/undiscussed tooling changes 😅 It always tends to produce quite a lot of work and research/work for all parties involved

[o-az]

Thank you for taking the time to review and share your feedback. Really appreciate it. I would like to write a longer reply to discuss some of the feedback you shared. I'm very curious to hear your thoughts. It will a short reply because I don't to take much of your time. If you don't mind keeping the pr open or I can convert it to a draft till then. It's also completely understandable if you want to close the pr - I would still like to hear your thoughts 😊

[kitten]

Hiya :wave: I realised this didn't actually get a follow-up comment. I'll close this for now, since as discussed, at the very least, it'd be hard to merge a large and non-atomic PR like this. Hope that makes sense ✌️ Still happy to chat of course. We also have a Discord channel for wonka now if that's easier