dst icon indicating copy to clipboard operation
dst copied to clipboard

Generics not working as expected when decorator uses ResolveLocalPath

Open myshkin5 opened this issue 2 years ago • 5 comments

When using a decorator with ResolveLocalPath set to true with generics, the path of the generic type is returned as the package where the type is located. I think it should always be empty string.

myshkin5 avatar Jun 18 '23 01:06 myshkin5

I have a failing test in #75 and I'm looking for pointers on how to fix it. I'm thinking that I need to inspect parent in fileDecorator.resolvePath. The problem I'm seeing is that I'm at least four levels down in decorateNode at that point (function type, params field list, field, then id).

myshkin5 avatar Jun 18 '23 01:06 myshkin5

I think it should always be empty string.

I haven't worked on this codebase for years so it's definitely possible I'm misunderstanding, but why? What's wrong with treating it as a local type?

If you're just trying to identify it as a generic type, then indicating this by having an empty string in the package path seems like a bad idea. e.g. when ResolveLocalPath==false, then you wouldn't be able to tell a generic type from a local type...

Or maybe I'm missing something?

dave avatar Jun 18 '23 07:06 dave

The problem I'm seeing is that I'm at least four levels down

Exactly... dst is concerned with the syntax, and is by design relatively dumb... it feels to me like determining whether a type is local or a generic type is perhaps something that requires a higher abstraction that dst is designed to do.

Remember, the ast package definitely can't tell you anything about types, and dst is supposed to replicate this as closely as possible.

The path resolving features are only supposed to make it possible to automatically manage the imports block. I feel that having generic types look just like local types will be ok...

Let me know what you think!

dave avatar Jun 18 '23 07:06 dave

I get it. I've implemented a type cache and I can repeatedly go back to the cache to see if a type lives in a context. I just have to keep track of what the context is.

I think this issue is still a concern -- using ResolveLocalPath causes type params to be reported incorrectly. Not sure anyone else is using ResolveLocalPath besides myself. I can update my PR to rip out the option if you would like. Or I can just leave it as is and get on with working around my issue.

Let me know how you would like to proceed. Thanks!

myshkin5 avatar Jun 19 '23 21:06 myshkin5

Use your work-around for now, but we'll keep the ticket open and if lots of people have the same problem, we'll look at fixing it... I would need to spend a while re-acclimatising myself with the codebase because I haven't worked on this project for years.

dave avatar Jun 19 '23 21:06 dave