search

tnc-ca-geo / animl-api

Backend for https://animl.camera
Other
4 stars 0 forks source link

TypeScript: Resolvers(-> Task): Update codegen to use mongoose.Types.ObjectID for ID scalar #202

Closed alukach closed 3 months ago

alukach commented 3 months ago

This PR instructs the codegen tooling that it's ok for our codebase to return objects with _id values of either string or mongoose.Types.ObjectId. It seems that both are equally stringifyable:

▶ node
Welcome to Node.js v18.17.1.
Type ".help" for more information.
> const mongoose = require ('mongoose')
undefined
> JSON.stringify({_id: new mongoose.Types.ObjectId()})
'{"_id":"6663529b4163c55e82765da8"}'

By allowing ourselves to return mongoose.Types.ObjectId objects, we can use the auto-generated types provided by mongoose, avoiding the need to create types from the schemas ourselves.

alukach commented 3 months ago

@nathanielrindlaub sorry for the confusion, I created this PR but left it in the draft state without reviewers as it was still a bit of a work in progress.

I agree that we don't want to create custom types for our interfaces. However, I have been hitting strange type errors with both the Task and Project schemas (both contain properties named type). I am honestly unsure if it is the type property that is the cause of the errors (or at least related), or if this is just a coincidence. In this PR, I've been able to get around the Task errors by letting GraphQL->TS codegen know that an _id can be either a string or ObjectId.

TS is mature enough that tools like Mongoose have good support for inferring types based on inputted schemas, however this isn't always perfect. Providing custom types to give it a hand is something we may need to reach for, but hopefully not.

With regards to the .d.ts file, this is a way to specify a global type file within TypeScript, so it doesn't need to be imported explicitely. This allows us to specify types for systems that we don't actually control. Pretty much the same this as in this convo, however in this case we're adding the types rather than @types/aws-lambda adding them.

alukach commented 3 months ago

@nathanielrindlaub I'm going to remove the mongodb-cursor-pagination types from this PR as we don't need them just yet, but that change will be coming down the pipe (once we start converting the schema files).