jarle / remix-adonisjs

Build fullstack Remix applications powered by AdonisJS
https://matstack.dev/remix-adonisjs
MIT License
47 stars 4 forks source link

Serializing Lucid Models #64

Open solomonhawk opened 3 months ago

solomonhawk commented 3 months ago

Hello πŸ‘‹

Thank you for all the work you've invested in putting this together. I have a question that perhaps isn't ultimately specific to the Adonis+Remix combo (moreso for lucid + client-rendered framework), but certainly rears it's head prominently once you get into returning models from loaders. I hope by posting it here other folks who run into the same thing might find the discussion useful.

Lucid models aren't type-safe. In practice, what this means is that when you query for a model in lucid and return it from a loader, remix picks up the type that lucid defines as a model's toJSON() method (which is just ModelObject, aka { [key: string]: any }):

// post_service.ts
class PostService {
  // typescript infers this as `(id: number) => Promise<Post>`
  async get(id: number) {
    return Post.getOrFail(id)
  }
}
// (some remix route)
export async function loader({ context, params }: LoaderFunctionArgs) {
  const postService = await context.make('post_service')

  return json({
    post: postService.get(Number(params.id))
  })
}

export default function PostShow() {
  // typescript infers this as `JsonifyObject<{ post: Post }>` which simplifies to `{ post: ModelObject }` due to lucid's typings
  const { post } = useLoaderData<typeof loader>()

  // ..
}

It may be tempting to just type-cast the useLoaderData hook return value as { post: Post }:

const { post } = useLoaderData() as { post: Post }

But that's not a great idea for a few reasons:

It seems sensible then to have some serialization logic somewhere that turns the result of a particular lucid query into a proper plain object. In that case, where's the best place to put that logic?

We could treat the service layer as a boundary, encapsulating the concern of using the ORM and then serializing the resulting data into plain objects. However, it may be beneficial to allow adonis service methods to return proper model class instances which callers can then operate on (in other adonis code).

My current thought is to manually handle serialization at the network boundary (i.e. in the loader, before returning the data). It's tricky to do right though since a service method that returns a Post may have preloaded certain associations or have annotated the resulting rows with additional properties (i.e. $extras) - i.e. any of the things lucid allows you to do when querying (which are mostly things that knex allows you to do).

As the author of this remix-adonisjs, I'm curious to get your thoughts on the matter.

Cheers πŸ‘‹

jarle commented 2 months ago

This is an interesting problem that I have encountered myself when building.

I ususally peek at the work done in the AdonisJS Inertia implementation to see how they handle these cases, and I see that they suggest casting the model to a simple object or adding DTO classes: https://docs.adonisjs.com/guides/views-and-templates/inertia#model-serialization

I personally create inlined DTOs where I extract the fields I need like this:

json({
  title: post.title,
  description: post.description,
})

A more elegant solution would be nice for this. Maybe it could be possible to augment the types for the Remix json method to give better typing with something like this that infers the model fields:

type ExtractModelFields<T> = {
  [K in keyof T as T[K] extends Function ? never : K extends `$${string}` ? never : K]: T[K]
}

It doesn't handle relationships. but might be a start :)

solomonhawk commented 2 months ago

Oh yeah the inertia serialization docs are informative. Between casting the model.serialize() and explicit DTO modeling I think I lean toward the more explicit pattern.

Manually serializing the objects on a per-case basis in loaders is clean and simple, but doesn't scale that well if you have many loaders and you're doing the same serialization over and over.

Depending on the use case I can see there being many different ways you might end up serializing a model so I shy away from the idea of a generalized type that extracts the model attributes. I also think it might be tricky to augment the types since json(..) makes use of Jsonify<T> under the hood and that pulls a type from an objects toJSON() method if it's present. It's certainly possible to monkey-patch something in there but it's not without some risk.

I ended up just defining presenters for each model+use case combination. What I don't like about it is that you still are in a situation where you need to know that PostWithAuthorDTO has to be used to serialize the data from PostService.getWithAuthor(..) (i.e. there's a coupling between the specific DTO and the service methods that I don't want to enforce in the service layer (again I think it's useful to just return real lucid model objects there for use in other parts of the adonis codebase).

I'll keep working with it and if I come up with something I really like I'm happy to share it back here.

Thank you for sharing your thoughts!