search

WebReflection / not-so-weak

Iterable WeakMap, WeakSet and WeakValue
ISC License
36 stars 1 forks source link

Improper type parameters #2

Open bogeeee opened 2 months ago

bogeeee commented 2 months ago

The type parameters aren't passed to the base class properly:

import {WSet, WKey, WValue} from 'not-so-weak';

type MyType = { myField: number };
const wv = new WValue<string, MyType>;
wv.set('value', {someInvalidType: 123} ); // Expected: get a compile error because i passed an invalid type. Actual: no error.

You should change the line in index.d.ts to export class WValue<K, V extends object> extends Map<K,V> {}

and the same for WSet and WKey

WebReflection commented 2 months ago

IIRC that's automatically generated via TS ... you should file a bug in TS? otherwise PR welcome, thanks!

WebReflection commented 2 months ago

actually ... it's not, so PR welcome, also I forgot the types in the package.json file ... apologies I don't use raw TS, I just use JSDoc TS and that never let me down, but clearly I'd love to have both worlds happy, as I usually do in all other projects of mine.

WebReflection commented 2 months ago

P.S. I personally value 100% code coverage and tests over any TS correctness but it's true that one doesn't need to exclude the other, whenever that's possible (so many projects with people mad at me from TS world ... I use JS and stuff just works there, sorry).

bogeeee commented 2 months ago

IIRC that's automatically generated via TS ... you should file a bug in TS? otherwise PR welcome, thanks!

Ok, how do i know which files are autogenerated and which not, if you commit all into git. Just saying. Cause i saw your duplicate ESM/CJS code and therefore really thought that you wrote the .d.ts by hand. Cause i don't see any single type information there in */index.js code nor jsdoc comments with type info !?. Just saying...very confusing.

So with that "unusual" project structure, it would be quicker if you fixed this yourself and make the library consuming TS guys a bit happier by that ;) Thaaaaaaanks !!

WebReflection commented 2 months ago

there's no duplicate code in here ... it's called dual module ... I've been around for 20+ years and dual module was my attempt to not break everyone expectations stuck in CommonJS world ... I use ESM though, daily, further, forever, if that makes sense ... please don't turn on me something I've done extensively for the community, thanks.

WebReflection commented 2 months ago

I know you might have landed in this project "by accident" but I maintain modules used, summing that up, 300M downloads per months (or was that weeks?) around the globe ... all dual modules with their cjs counter-folder.

bogeeee commented 2 months ago

Ok ok. Just take my hint / improvement that it's confusing that you look at the git project and see the 2 folders: esm and cjs. They look almost exactly the same by content but and one of them is a generated build artifact and one is the source code. So one would usually gitignore the build artifact.