Compilation failure

[DanielReid]

Hi,

On attempting to use your (nice-looking!) library, I get a compilation failure:

node_modules/csv42/lib/types/types.d.ts:2:13 - error TS2456: Type alias 'NestedObject' circularly references itself.

2 export type NestedObject = Record<string, NestedObject>;
              ~~~~~~~~~~~~

It appears that the @ts-ignore you put in your types.ts is not added to the compiled types.d.ts.

Some interesting discussion around this behaviour on the typescript repo: https://github.com/microsoft/TypeScript/issues/38628 Not sure what the best solution would be.

I was surprised by this warning since recursive types are possible in newer Typescript. Changing the definition to export type NestedObject = {[key: string]:NestedObject}; does work, so I expect there's something special about Record here that's throwing soot in the dinner.

[josdejong]

Thanks Daan, and thanks for reporting this TypeScript issue.

I think a better definition would indeed be something like your proposal, though the values can be an object or anything:

export type NestedObject = { [key: string]: NestedObject | unknown }

This requires changes at the places where NestedObject is used, and I haven't figured out a neat solution that does not involve a lot of plumbing and type casting just to make TS happy. Help improving this would very welcome!

A workaround to fix keeping // @ts-ignore in place would be great for the time being.

[josdejong]

Hmm, it may work out like that, when I trade a // @ts-ignore at the type definition for a couple of them in the implementation. Let me try that out.

[josdejong]

The workaround mentioned here works, that was very helpful: https://github.com/microsoft/TypeScript/issues/38628#issuecomment-1378644305

I've published that workaround for the time being in v3.0.4. Can you give that a try Daan?

[DanielReid]

Hi Jos,

That works indeed! As the thread notes this isn't a 100% future-proof way of dealing with it (future Typescript releases might strip jsdocs as well), but I approve of the pragmatic approach if the alternative is a lot of work. Thanks for the quick fix. If I can find some spare time and remind myself I might look into the typing a bit myself, but you know how that goes.

[josdejong]

Good to hear it works. It's indeed a shaky solution. I will open this issue again as a reminder to find a better solution soon.

[josdejong]

I've done some refactoring to improve the TypeScript support, see https://github.com/josdejong/csv42/commit/9ec74ae4ffc782a92210e6cc9234835c3ef345da

There is no need anymore for the // @ts-ignore. The API now has generics, which makes it easier to define value getters with getValue. The type of NestedObject is changed, it is now mostly used internally and only needed for value setters setValue. And internally I introduced another type MergedObject used inside the collectNestedPaths function. I'm quite happy with the result!

Can you give v4.0.0 a try @DanielReid ?

[DanielReid]

:+1: Works like a charm!

[josdejong]

😎 thanks for checking!