refactor: clean up type casting in region resolution#10504
Conversation
wandamora
force-pushed
the
morawand-refactor-endpoint-types
branch
from
82b9be1 to
6320284
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces EventTriggerResolutionTarget and BlockingTriggerResolutionTarget interfaces to replace the use of any and complex intersection types during region resolution. Feedback identifies that BlockingTriggerResolutionTarget is missing a project property and notes an unsafe type cast to backend.Endpoint which circumvents TypeScript safety checks.
| endpoint: backend.Endpoint & backend.BlockingTriggered, | ||
| ): string { | ||
| function resolveRegionForBlockingTrigger(endpoint: TriggerResolutionTarget): string { | ||
| if (!endpoint.blockingTrigger) { |
There was a problem hiding this comment.
I don't love this. Like we should only be calling this for blocking triggers and this was enforced by the old code right with a type of backend.Endpoint & backed.blockingTriggered
This seems to open us up to subtle bugs in the future where we call the wrong region resolution method and then incorrectly fall back to the default function region.
There was a problem hiding this comment.
The root problem seems to be that we're trying to construct a single endpoint ish object that has everything the method needs. But we don't really have to do that right? I realize the overall thing is working w/ endpoints and backends. But we don't have to only pass those around.
What if these methods just took individual args for only the info the needed.
E.g. this one only needs the blockingTrigger which already has a type and the resolveRegionForEventTrigger only needs eventTrigger and project?
I guess the issue with that is nominally we need a single interface if we want to in our next refactoring move these methods onto the service so we just call
service.resolveRegionForEventTrigger
But we could just also provide project for blocking triggers even though it's not used?
Thoughts?
3 participants
Description
Introduces a minimal interface for resolving default regions for better type-safety. This includes refactors in both prepare.ts and ailogic.ts.