Global models and interface hierarchy #145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sideeffect response models are unique because they require a concrete RenderBase of a mounted controller, but the functions themselves can still be defined at the level of the superclass. Previously we only had a 1:1 relationship between an action function's metadata and the response model - but in cases of sideeffects this resulted in a last-win assignment of the render model to the `sideeffect` property. Our global controllers typehinted incorrectly in this situation. This commit solves the issue by registering action render models for each concrete controller that's registered. Global controllers will do a union of each identity, whereas the controller owned implementation itself will only use its more specific sideeffect render.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a pretty significant change to the generation pipeline for TS interfaces. We remove our previous OpenAPI intermediary representation in preference of doing controller introspection manually. This gives us more control to:
When all is said and done, this creates a superset of the previous auto-generated files. Client are able to use the previous model definitions if they wish. But they can now optionally interact with all of the classes that are defined in the Python codebase, which furthers our goal of unified typing across frontend and backend.
Migration Guide
Enums
Enums have changed in definition from removing underscores to with proper indentations. This should align with their keys in the original Python. Previously we only had access to the values.
APIException
Exceptions no longer have the "Exception" suffix added to them in the frontend. You should be able to import them with just their exception name.
Required Fields in TS Interfaces
Required fields are a bit hard to reason about when we're thinking about client->server->client syncing. For client->server syncing, if a field has a default in Pydantic, we want it to be optional for clients to send (so it defaults to the server value). But for server->client models that's not true. We know it will be provided within the JSON payload.
We previously tried to be "smart" about this behavior, where if a model is used as a server->client model only we would typehint it as having all values. But once an action uses this same model as a client->server type, the same interface would become optional. This worked for simple cases but resulted in instability for larger projects where one change in usage can cascade through the type definitions. The new version makes this behavior consistent all the time by typehinting these models as optional.
NOTE: If you want to provide default values, but still want the interfaces typehinted as required in TypeScript, you can do so by providing the defaults as part of a model
@validation.