-
Notifications
You must be signed in to change notification settings - Fork 0
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
Geofence config #69
Geofence config #69
Conversation
import type { WithKey } from "@/util"; | ||
import Dataloader from "dataloader"; | ||
import type { Request } from "express"; | ||
import { GraphQLError } from "graphql/error"; | ||
import { sql } from "./postgres"; | ||
|
||
export default (_: Request) => | ||
new Dataloader<string, Location>(async keys => { | ||
const rows = await sql<WithKey<Location>[]>` | ||
new Dataloader<string, Location & Geofence>(async keys => { |
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.
shouldnt need Location & Geofence
right? since type Location
below has a geofence
property. or maybe im misreading that bit
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.
No, you're reading right but these types are a little misleading. We aren't grabbing a full Location, but rather a smaller subset: just the scalar properties and foreign key references. The Location resolver (capital L Location.ts) then uses the foreign keys to resolve those objects (if the user asks for them).
In the case of geofence, it is an object and it is easier to build that object in js than sql. So we're just flattening its fields when we grab it from sql, and then bundling them into an object in the Location resolver.
Kinda janky. Definitely a little misleading. The newer stuff (Checklist) does all of this a little better.
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.
No description provided.