Type model information in SearchDrawer #5597
Conversation
✅ Deploy Preview for inventree ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
matmair
changed the title
There was a problem hiding this comment.
Generally, I would define one interface for each Model, extending from a Generic model interface. Next create a union type. Then everywhere you need any model, use the union type. Otherwise you can type guard, checking model.type. That way you could define the appropriate type for each renderer, and have type safety there. And generally, are you aware of typescript generics? (I'm not sure if you need them in this scenario, but they get pretty handy)
|
I still have to learn the patterns @wolflu05 mentioned and rewrite a bit of code, this will take at least till the weekend |
|
@wolflu05 is this the way you meant? I researched generics but i am not sure how one would use them here. |
|
Looks much better. The only thing we could do is add typing for each model, instead of using any everywhere. But to keep them in sync, maybe it would be good to generate them via a Python script. But then we are again at the point where it would make sense to generate them from the api schema, which is another topic. I would suggest doing this manually for now so that we have the types already and can auto generate them at a later point. You're right, you don't need generics here, but they are good to know if you deal with function that have related arguments/return types, so they can be used instead of any. I first thought this would be useful for the model type input and the instance input. But then I realized that we don't have any typing for the models, and therefore also no map from model type keys to the actual model type |
|
@SchrodingersGat this is ready for merge from my POV |
|
Cheers @matmair |
This PR moves model information out from the SearchDrawer and in a reusable typed format. That way it can be used in other places and the InstanceRender can not be supplied with unknown models.
Made this to make #5559 simpler