Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary duplicated types at Linx commerce App #801

Closed
viktormarinho opened this issue Aug 23, 2024 · 2 comments · Fixed by #832
Closed

Unnecessary duplicated types at Linx commerce App #801

viktormarinho opened this issue Aug 23, 2024 · 2 comments · Fixed by #832
Assignees

Comments

@viktormarinho
Copy link
Contributor

Linx app types/ contains a lot of duplicated types, we should Merge those

@Samuel-Brito19
Copy link
Contributor

Hi! Can you provide more information about the issue? I'm interest.

@viktormarinho
Copy link
Contributor Author

@Samuel-Brito19

The Linx app utils/types directory (https://github.com/deco-cx/apps/tree/main/linx/utils/types) currently contains a lot of those JSON.ts files.

What happened here is that i used a tool that can generate typescript types from a JSON object (quicktype.io) to map every return of the Linx APIs.

So, for example, when creating the auctionDetail loader, i started by hitting the Linx Endpoint for auction details, copying the json response, pasting it into Quicktype then copying the generated TS Types to the project, putting it in the auctionDetailJSON.ts file.

The problem here is that a lot of these types were shared between different endpoints, and i ended up having a bunch of duplicated types. I already started deduplicating some of those types, but there's a lot of duplicated ones still. For example, i created the facets.ts file and made it the only Facet interface in the linx types folder:

Captura de Tela 2024-09-02 às 14 13 46

Before, i had a lot of Facet types. But there are more types that can be merged. For example, this Description type, that really should have another name haha, probably something like LinxMetadata that better reflects it.

Captura de Tela 2024-09-02 às 14 14 26

I want to merge all of these types, it will make the development a lot easier, specially when writing Linx type transformations to Schema.org, and overall improve the correctness of our code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants