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

Add type definitions for API responses #367

Closed
eyeseast opened this issue Dec 9, 2023 · 4 comments · Fixed by #423
Closed

Add type definitions for API responses #367

eyeseast opened this issue Dec 9, 2023 · 4 comments · Fixed by #423
Labels
enhancement New feature or request

Comments

@eyeseast
Copy link
Collaborator

eyeseast commented Dec 9, 2023

We did this with add-ons. We should add coverage for the rest of the data we pull in.

It's possible this will make the src/structure folder obsolete.

@eyeseast eyeseast added the enhancement New feature or request label Dec 9, 2023
@eyeseast eyeseast added this to the SvelteKit milestone Dec 10, 2023
@allanlasser
Copy link
Member

Adding to this, consolidating the fixtures and mocked data handlers into the /api directory will help keep everything close-knit.

@allanlasser
Copy link
Member

allanlasser commented Dec 11, 2023

And if the API supports OpenAPI, we can generate our response types automatically:

@eyeseast
Copy link
Collaborator Author

Should our types live in .d.ts files? Or in Svelte components? Somewhere else?

@allanlasser
Copy link
Member

allanlasser commented Dec 11, 2023

I'll admit this isn't something I've understood well, but I think this SO answer is pretty good: https://stackoverflow.com/a/29197665/4256689

In short:

  • Types that augment existing .js should go in .d.ts. This gives our imports from JS files type-checking. .d.ts is also for cases where we're not intending to compile down to working JS. Typed API responses, where there's no code we're directly referencing, seems like a good case for this kind of file.
  • We can still define and export types from Svelte components when they make sense. Defining the type for an event handling function prop, for example.

@allanlasser allanlasser linked a pull request Feb 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants