fix: translation fixes and perf improvements

src/datatype/index.js

@@ -2,7 +2,7 @@ import { validate } from '@comapeo/schema'
 import { getTableConfig } from 'drizzle-orm/sqlite-core'
 import { eq, inArray, sql } from 'drizzle-orm'
 import { randomBytes } from 'node:crypto'
-import { noop, deNullify } from '../utils.js'
+import { noop, mutatingDeNullify } from '../utils.js'
 import { NotFoundError } from '../errors.js'
 import { TypedEmitter } from 'tiny-typed-emitter'
 import { parse as parseBCP47 } from 'bcp-47'
@@ -105,7 +105,7 @@ export class DataType extends TypedEmitter {
    * @param {TTable} opts.table
    * @param {TDataStore} opts.dataStore
    * @param {import('drizzle-orm/better-sqlite3').BetterSQLite3Database} opts.db
-   * @param {import('../translation-api.js').default['get']} opts.getTranslations
+   * @param {import('../translation-api.js').default['get']} [opts.getTranslations]
    * @param {(versionId: string) => Promise<string>} opts.getDeviceIdForVersionId
    */
   constructor({
@@ -232,7 +232,7 @@ export class DataType extends TypedEmitter {
     await this.#dataStore.indexer.idle()
     const result = this.#sql.getByDocId.get({ docId })
     if (result) {
-      return this.#translate(deNullify(result), { lang })
+      return this.#mutatingAddDerivedFields(result, { lang })
     } else if (mustBeFound) {
       throw new NotFoundError()
     } else {
@@ -243,61 +243,79 @@ export class DataType extends TypedEmitter {
   /**
    * @param {string} versionId
    * @param {{ lang?: string }} [opts]
-   * @returns {Promise<TDoc>}
+   * @returns {Promise<TDoc & DerivedDocFields>}
    */
   async getByVersionId(versionId, { lang } = {}) {
     const result = await this.#dataStore.read(versionId)
     if (result.schemaName !== this.#schemaName) throw new NotFoundError()
-    return this.#translate(deNullify(result), { lang })
+    return this.#mutatingAddDerivedFields(result, { lang })
   }
 
   /**
-   * @param {any} doc
+   * @param {any} doc - not typesafe - this should be a MapeoDoc with nullable fields in place of optional fields, read from SQLite
+   * @param {{ lang?: string }} [opts]
    * @returns {Promise<TDoc & DerivedDocFields>}
    */
-  async #addCreators(doc) {
-    doc.createdBy = await this.#getDeviceIdForVersionId(doc.originalVersionId)
-    doc.updatedBy =
+  async #mutatingAddDerivedFields(doc, { lang } = {}) {
+    mutatingDeNullify(doc)
+    const createdByPromise = this.#getDeviceIdForVersionId(
+      doc.originalVersionId
+    )
+    const updatedByPromise =
       doc.originalVersionId === doc.versionId
-        ? doc.createdBy
-        : await this.#getDeviceIdForVersionId(doc.versionId)
+        ? createdByPromise
+        : this.#getDeviceIdForVersionId(doc.versionId)
+    if (lang) {
+      await this.#mutatingAddTranslations(doc, { lang })
+    }
+    const [createdBy, updatedBy] = await Promise.all([
+      createdByPromise,
+      updatedByPromise,
+    ])
+    doc.createdBy = createdBy
+    doc.updatedBy = updatedBy
     return doc
   }
 
   /**
    * @param {any} doc
-   * @param {{ lang?: string }} [opts]
-   * @returns {Promise<TDoc & DerivedDocFields>}
+   * @param {{ lang: string }} opts
+   * @returns {Promise<any>}
    */
-  async #translate(doc, { lang } = {}) {
-    await this.#addCreators(doc)
-
-    if (!lang) return doc
+  async #mutatingAddTranslations(doc, { lang }) {
+    if (!this.#getTranslations) return doc
 
     const { language, region } = parseBCP47(lang)
     if (!language) return doc
-    const translatedDoc = JSON.parse(JSON.stringify(doc))
 
-    const value = {
+    const translations = await this.#getTranslations({
       languageCode: language,
       docRef: {
-        docId: translatedDoc.docId,
-        versionId: translatedDoc.versionId,
+        docId: doc.docId,
+        versionId: doc.versionId,
       },
-      docRefType: translatedDoc.schemaName,
-      regionCode: region !== null ? region : undefined,
-    }
-    let translations = await this.#getTranslations(value)
-    // if passing a region code returns no matches,
-    // fallback to matching only languageCode
-    if (translations.length === 0 && value.regionCode) {
-      value.regionCode = undefined
-      translations = await this.#getTranslations(value)
-    }
+      docRefType: doc.schemaName,
+    })
 
+    /** @type {Set<string>} */
+    const translationsWithMatchingRegion = new Set()
     for (const translation of translations) {
       if (typeof getProperty(doc, translation.propertyRef) === 'string') {
-        setProperty(doc, translation.propertyRef, translation.message)
+        const isMatchingRegion = region
+          ? translation.regionCode === region
+          : false
+        // Prefer translations with a matching region code, but fall back to
+        // translations without a region code if no matching region code has
+        // been found yet for this propertyRef
+        if (
+          isMatchingRegion ||
+          !translationsWithMatchingRegion.has(translation.propertyRef)
+        ) {
+          setProperty(doc, translation.propertyRef, translation.message)
+        }
+        if (isMatchingRegion) {
+          translationsWithMatchingRegion.add(translation.propertyRef)
+        }
       }
     }
 
@@ -316,7 +334,7 @@ export class DataType extends TypedEmitter {
       ? this.#sql.getManyWithDeleted.all()
       : this.#sql.getMany.all()
     return await Promise.all(
-      rows.map((doc) => this.#translate(deNullify(doc), { lang }))
+      rows.map((doc) => this.#mutatingAddDerivedFields(doc, { lang }))
     )
   }
 
@@ -421,7 +439,7 @@ export class DataType extends TypedEmitter {
         .from(this.#table)
         .where(inArray(this.#table.docId, [...docIds]))
         .all()
-        .map((doc) => this.#addCreators(deNullify(doc)))
+        .map((doc) => this.#mutatingAddDerivedFields(doc))
     )
     updatedDocs.then((docs) => this.emit('updated-docs', docs))
   }

src/mapeo-project.js

@@ -296,21 +296,18 @@ export class MapeoProject extends TypedEmitter {
         dataStore: this.#dataStores.data,
         table: observationTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId,
       }),
       track: new DataType({
         dataStore: this.#dataStores.data,
         table: trackTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId,
       }),
       remoteDetectionAlert: new DataType({
         dataStore: this.#dataStores.data,
         table: remoteDetectionAlertTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId,
       }),
       preset: new DataType({
@@ -331,44 +328,36 @@ export class MapeoProject extends TypedEmitter {
         dataStore: this.#dataStores.config,
         table: projectSettingsTable,
         db: sharedDb,
-        getTranslations,
         getDeviceIdForVersionId,
       }),
       coreOwnership: new DataType({
         dataStore: this.#dataStores.auth,
         table: coreOwnershipTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId: () => Promise.resolve(''),
       }),
       role: new DataType({
         dataStore: this.#dataStores.auth,
         table: roleTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId: () => Promise.resolve(''),
       }),
       deviceInfo: new DataType({
         dataStore: this.#dataStores.config,
         table: deviceInfoTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId: () => Promise.resolve(''),
       }),
       icon: new DataType({
         dataStore: this.#dataStores.config,
         table: iconTable,
         db,
-        getTranslations,
         getDeviceIdForVersionId,
       }),
       translation: new DataType({
         dataStore: this.#dataStores.config,
         table: translationTable,
         db,
-        getTranslations: () => {
-          throw new Error('Cannot get translation for translations')
-        },
         getDeviceIdForVersionId,
       }),
     }

src/translation-api.js

@@ -35,7 +35,7 @@ export default class TranslationApi {
         docs.map((doc) => this.index(doc))
       })
       .catch((err) => {
-        throw new Error(`error loading Translation cache: ${err}`)
+        console.error(`error loading Translation cache: ${err}`)
       })
   }
 

src/types.ts

@@ -74,25 +74,6 @@ export type CoreOwnershipWithSignaturesValue = Omit<
   Exclude<keyof MapeoCommon, 'schemaName'>
 >
 
-type NullToOptional<T> = SetOptional<T, NullKeys<T>>
-type RemoveNull<T> = {
-  [K in keyof T]: Exclude<T[K], null>
-}
-
-type NullKeys<Base> = NonNullable<
-  // Wrap in `NonNullable` to strip away the `undefined` type from the produced union.
-  {
-    // Map through all the keys of the given base type.
-    [Key in keyof Base]: null extends Base[Key] // Pick only keys with types extending the given `Condition` type.
-      ? // Retain this key since the condition passes.
-        Key
-      : // Discard this key since the condition fails.
-        never
-
-    // Convert the produced object into a union type of the keys which passed the conditional test.
-  }[keyof Base]
->
-
 /**
  * Replace an object's `Buffer` values with `string`s. Useful for serialization.
  */
@@ -106,7 +87,17 @@ export type MapBuffers<T> = {
  * top-level optional props set to `null`) to the original types in
  * @comapeo/schema
  */
-export type NullableToOptional<T> = Simplify<RemoveNull<NullToOptional<T>>>
+export type NullableToOptional<T> = Simplify<
+  {
+    [K in keyof T as null extends T[K] ? K : never]?: Exclude<T[K], null>
+  } & {
+    [K in keyof T as null extends T[K] ? never : K]: T[K]
+  }
+>
+export type OptionalToNullable<T> = Simplify<{
+  [K in keyof T]-?: T[K] | (undefined extends T[K] ? null : never)
+}>
+
 export type KeyPair = {
   publicKey: PublicKey
   secretKey: SecretKey

src/utils.js

@@ -87,7 +87,6 @@ export function isDefined(value) {
  * @param {T} obj
  * @returns {import('./types.js').NullableToOptional<T>}
  */
-
 export function deNullify(obj) {
   /** @type {Record<string, any>} */
   const objNoNulls = {}
@@ -97,6 +96,26 @@ export function deNullify(obj) {
   return /** @type {import('./types.js').NullableToOptional<T>} */ (objNoNulls)
 }
 
+/**
+ * __Mutating__
+ * When reading from SQLite, any optional properties are set to `null`. This
+ * converts `null` back to `undefined` to match the input types (e.g. the types
+ * defined in @comapeo/schema)
+ * @template {{}} T
+ * @param {T} obj
+ * @returns {import('./types.js').NullableToOptional<T>}
+ */
+export function mutatingDeNullify(obj) {
+  for (const key of Object.keys(obj)) {
+    // @ts-expect-error
+    if (obj[key] === null) {
+      // @ts-expect-error
+      obj[key] = undefined
+    }
+  }
+  return /** @type {import('./types.js').NullableToOptional<T>} */ (obj)
+}
+
 /**
  * @template {import('@comapeo/schema').MapeoDoc & { forks?: string[], createdBy?: string, updatedBy?: string }} T
  * @param {T} doc

test-types/data-types.ts

@@ -75,7 +75,7 @@ Expect<Equal<Observation & DerivedDocFields, typeof observationByDocId>>
 const observationByVersionId = await mapeoProject.observation.getByVersionId(
   'abc'
 )
-Expect<Equal<Observation, typeof observationByVersionId>>
+Expect<Equal<Observation & DerivedDocFields, typeof observationByVersionId>>
 
 mapeoProject.observation.on('updated-docs', (docs) => {
   Expect<Equal<Observation[], typeof docs>>
@@ -104,7 +104,7 @@ const trackByDocId = await mapeoProject.track.getByDocId('abc')
 Expect<Equal<Track & DerivedDocFields, typeof trackByDocId>>
 
 const trackByVersionId = await mapeoProject.track.getByVersionId('abc')
-Expect<Equal<Track, typeof trackByVersionId>>
+Expect<Equal<Track & DerivedDocFields, typeof trackByVersionId>>
 
 mapeoProject.track.on('updated-docs', (docs) => {
   Expect<Equal<Track[], typeof docs>>
@@ -133,7 +133,7 @@ const presetByDocId = await mapeoProject.preset.getByDocId('abc')
 Expect<Equal<Preset & DerivedDocFields, typeof presetByDocId>>
 
 const presetByVersionId = await mapeoProject.preset.getByVersionId('abc')
-Expect<Equal<Preset, typeof presetByVersionId>>
+Expect<Equal<Preset & DerivedDocFields, typeof presetByVersionId>>
 
 mapeoProject.preset.on('updated-docs', (docs) => {
   Expect<Equal<Preset[], typeof docs>>
@@ -159,7 +159,7 @@ const fieldByDocId = await mapeoProject.field.getByDocId('abc')
 Expect<Equal<Field & DerivedDocFields, typeof fieldByDocId>>
 
 const fieldByVersionId = await mapeoProject.field.getByVersionId('abc')
-Expect<Equal<Field, typeof fieldByVersionId>>
+Expect<Equal<Field & DerivedDocFields, typeof fieldByVersionId>>
 
 mapeoProject.field.on('updated-docs', (docs) => {
   Expect<Equal<Field[], typeof docs>>