edgedb / edgedb-js

The official TypeScript/JS client library and query builder for EdgeDB
https://edgedb.com
Apache License 2.0
514 stars 65 forks source link

Allow to modify select results #644

Closed ssendev closed 1 year ago

ssendev commented 1 year ago

Something like this:

e.select(e.File, () => ({
  file_path: true,
  name: true,
  width: true,
  height: true,
}), async (file) => ({ 
  url: await getUrl(file),
  width: file.width,
  height: file.height,
}))
// or maybe
e.select(e.File, () => ({ file_path: true })).map(async (file) => ({ url: await(getUrl(file)) }))

Why?

Bacause when returning a deeply nested data structure it's verbose and annoying to replicate it all.

The alternative is a recursive function that e.g. finds all objects with a file_path and inserts the url. The problem with this is that the types don't match anymore. Its also requires writing a function to find the things one wants to modify.

This is the workaround I'm currently using but it's not exactly pretty.

type File {
  required file_path: str;
  required name: str;
  width: int32;
  height: int32;
  property url := ""; # this is never queried and for TypeScript only
}
setUrls(e.select(url.File, () => ({
  file_path: true,
  name: true,
  width: true,
  height: true,
} as unknown as {
  url: true, // without the computed the type would be inferred as never
  width: true,
  height: true
}))

edit: a better way is

setUrls(e.select(url.File, () => ({
  file_path: true,
  name: true,
  width: true,
  height: true,
} as unknown as {
  url: typeof e.str, // now the computed property is not necessary
  width: true,
  height: true
}))
scotttrinh commented 1 year ago

It's possible that I'm misunderstanding since I don't know the full schema or downstream types, but I think you can get what you're after by using the built-in Promise.then function when running the query. Something like:

e.select(url.File, () => ({
  file_path: true,
  name: true,
  width: true,
  height: true,
}).run(client).then((file) => ({ 
  url: await getUrl(file),
  width: file.width,
  height: file.height,
}));

If you can provide more detail about what getUrl is doing maybe we can provide some guidance around doing this directly in the query.

ssendev commented 1 year ago

Yes if its just one level it's no problem but if its deeply nested it's required to do the whole thing twice. Here is a little more of my motivating example:

export const showFileProps = {
  width: true,
  height: true,
  key: true,
  name: true,
  format: true,
  size: true,
  meta: true,
} as unknown as { url: true; width: true; height: true }

export const imgFormatCard = {
  w: 464,
  h: 261,
  fit: 'Cover',
  format: 'JPG',
} as const

export function fileUrls<T>(
  data: T,
  formats: Record<string, ResizeOptions>,
  edit = false
) {
  const promises: Promise<unknown>[] = []

  fileUrlsInner(data, formats, promises, edit)

  return Promise.all(promises).then(() => data)
}

function fileUrlsInner(
  data: unknown,
  formats: Record<string, ResizeOptions>,
  promises: Promise<unknown>[],
  edit: boolean
): void {
  if (!data) {
    return
  } else if (data instanceof Array) {
    for (const d of data) {
      fileUrlsInner(d, formats, promises, edit)
    }
  } else if (typeof data === 'object') {
    for (const key in data) {
      const item = data[key as keyof typeof data] as unknown
      let format: ResizeOptions
      if (
        item &&
        typeof item === 'object' &&
        (item as any).key &&
        (format = formats[key])
      ) {
        promises.push(resizedUploadUrl(item, format, true, !edit))
        continue
      }

      fileUrlsInner(item, formats, promises, edit)
    }
  }
}

async function getit(url: string) {
  const data = await e
    .params({ url: e.str }, (p) =>
      e.select(e.Url, (url) => ({
        filter_single: { path: p.url },
        knowledge: e.assert_single(
          e.select(url.knowledge, () => ({
            id: true,
            titel: true,
            text: true,
            files: showFileProps,
          }))
        ),
        kategorie: e.assert_single(
          e.select(url.knowledge_kategorien, (kategorie) => ({
            id: true,
            name: true,
            icon: true,
            color: true,
            list_order: true,
            bild: e.select(kategorie.bild, () => showFileProps),
            parent: e.select(kategorie.parent, (parent) => ({
              name: true,
              icon: true,
              color: true,
              url_path: parent.url.path,
              bild: showFileProps,
              parent: e.select(parent.parent, (parent) => ({
                name: true,
                icon: true,
                color: true,
                url_path: parent.url.path,
                bild: showFileProps,
              })),
            })),
            children: e.select(kategorie.children, (child) => ({
              name: true,
              icon: true,
              color: true,
              bild: showFileProps,
              url_path: child.url.path,
            })),
            knowledge: e.select(kategorie.knowledge, (child) => ({
              titel: true,
              url_path: child.url.path,
            })),
          }))
        ),
      }))
    )
    .run(edgeClient, { url: url })

  await fileUrls(data, { bild: imgFormatCard }, true)

  return data
}

Here replacing the fileUrls call with reconstructing the whole thing gets boring fast especially since Array.map is not async and requires even more boilerplate.

ssendev commented 1 year ago

If you can provide more detail about what getUrl is doing maybe we can provide some guidance around doing this directly in the query.

Here getUrl returns a signed url with the image rescaled to the specified size. But in other places can also be something like https://min.io/docs/minio/linux/developers/javascript/API.html#presignedGetObject so it's not practical to do it in the db.

and while here it's just one string property it might be desirable to return a more complex type

scotttrinh commented 1 year ago

I think I understand the ask now, but I don't believe it makes sense to implement this as part of the query builder. There are a few technical reasons why this just isn't possible (composed queries might not select the same properties, so the modifier callback would be invalid), and further more, the purpose of the query builder is to strictly build EdgeQL queries in a type safe way. Once we have to start tracking and applying side-effects, we're straying from that core mission.

There are tools like Zod that can help convert your various IO layers into your domain model and have first class support for composing things like this and async transformers. Thanks for the suggestion and for providing detailed feedback and use cases. I bet this will be helpful to others, even if it's not a feature they'll find available directly in the query builder itself.