-
-
Notifications
You must be signed in to change notification settings - Fork 684
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
Orval generator POC #2724
Orval generator POC #2724
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
Can we put this (the actual usage) behind a feature flag so that we can turn it on/off at any time. This is a very dangerous change so I'd like to be on the safe side. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense. For types I'm happy to start right away. For the API integration I think we should wait until we can validate properly and over time. We can for example try the fetchers on new features, put them behind feature flags and try it out as we are developing instead of modifying existing endpoints.
This way, we'll find edge cases while we are developing, and we can figure out if the generator provides a pleasant experience, or if we are just moving around the technical debt.
…enerator-for-frontend
@FredrikOseberg @kwasniew About putting it behind a flag: I only substituted 2 OpenAPI getters that where used before (get features and get features archive). All model/Schema names where compatible. We still have Files that where using previous auto-generated fetcher that I refactored:
|
…enerator-for-frontend
…enerator-for-frontend
[strategyId, uiConfig.flags], | ||
fetchSegments, | ||
url, | ||
() => (uiConfig.flags?.SE ? fetchSegments(url) : []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tried this approach in change requests and it didn't work. Please check SWR recommended approach: https://swr.vercel.app/docs/conditional-fetching. We have our own hook useConditionalSWR encapsulating the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Can be merged after addressing comments.
Couple of questions to cover everything.
|
…enerator-for-frontend
There is a comment in
We don't use methods/actions yet. For now we only use generated types. |
About the changes
POC for #2715
Important files
frontend/orval.config.js
frontend/src/openapi/fetcher.ts