Apply code review suggestions

bluesky-social · Nov 1, 2023 · 6ad0d0d · 6ad0d0d
1 parent 404e3f5
commit 6ad0d0d
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 50 deletions.
diff --git a/packages/lex-cli/src/util.ts b/packages/lex-cli/src/util.ts
@@ -1,6 +1,6 @@
 import fs from 'fs'
 import { join } from 'path'
-import { lexiconDoc, LexiconDoc } from '@atproto/lexicon'
+import { parseLexiconDoc, LexiconDoc } from '@atproto/lexicon'
 import { ZodError, ZodFormattedError } from 'zod'
 import chalk from 'chalk'
 import { GeneratedAPI, FileDiff } from './types'
@@ -41,8 +41,7 @@ export function readLexicon(path: string): LexiconDoc {
     typeof (obj as LexiconDoc).lexicon === 'number'
   ) {
     try {
-      lexiconDoc.parse(obj)
-      return obj as LexiconDoc
+      return parseLexiconDoc(obj)
     } catch (e) {
       console.error(`Invalid lexicon`, path)
       if (e instanceof ZodError) {

diff --git a/packages/lexicon/src/lexicons.ts b/packages/lexicon/src/lexicons.ts
@@ -1,12 +1,9 @@
-import { ZodError } from 'zod'
 import {
   LexiconDoc,
-  lexiconDoc,
   LexRecord,
   LexXrpcProcedure,
   LexXrpcQuery,
   LexUserType,
-  LexiconDocMalformedError,
   LexiconDefNotFoundError,
   InvalidLexiconError,
   ValidationResult,
@@ -32,7 +29,7 @@ export class Lexicons {
   docs: Map<string, LexiconDoc> = new Map()
   defs: Map<string, LexUserType> = new Map()
 
-  constructor(docs?: unknown[]) {
+  constructor(docs?: LexiconDoc[]) {
     if (docs?.length) {
       for (const doc of docs) {
         this.add(doc)
@@ -43,37 +40,19 @@ export class Lexicons {
   /**
    * Add a lexicon doc.
    */
-  add(doc: unknown): void {
-    if (process.env.NODE_ENV !== 'production') {
-      try {
-        lexiconDoc.parse(doc)
-      } catch (e) {
-        if (e instanceof ZodError) {
-          throw new LexiconDocMalformedError(
-            `Failed to parse schema definition ${
-              (doc as Record<string, string>).id
-            }`,
-            doc,
-            e.issues,
-          )
-        } else {
-          throw e
-        }
-      }
-    }
-    const validatedDoc = doc as LexiconDoc
-    const uri = toLexUri(validatedDoc.id)
+  add(doc: LexiconDoc): void {
+    const uri = toLexUri(doc.id)
     if (this.docs.has(uri)) {
       throw new Error(`${uri} has already been registered`)
     }
 
     // WARNING
     // mutates the object
     // -prf
-    resolveRefUris(validatedDoc, uri)
+    resolveRefUris(doc, uri)
 
-    this.docs.set(uri, validatedDoc)
-    for (const [defUri, def] of iterDefs(validatedDoc)) {
+    this.docs.set(uri, doc)
+    for (const [defUri, def] of iterDefs(doc)) {
       this.defs.set(defUri, def)
     }
   }

diff --git a/packages/lexicon/src/types.ts b/packages/lexicon/src/types.ts
@@ -415,16 +415,9 @@ export function isDiscriminatedObject(
   return discriminatedObject.safeParse(value).success
 }
 
-export class LexiconDocMalformedError extends Error {
-  constructor(
-    message: string,
-    public schemaDef: unknown,
-    public issues?: z.ZodIssue[],
-  ) {
-    super(message)
-    this.schemaDef = schemaDef
-    this.issues = issues
-  }
+export function parseLexiconDoc(v: unknown): LexiconDoc {
+  lexiconDoc.parse(v)
+  return v as LexiconDoc
 }
 
 export type ValidationResult =

diff --git a/packages/lexicon/tests/_scaffolds/lexicons.ts b/packages/lexicon/tests/_scaffolds/lexicons.ts
@@ -1,4 +1,6 @@
-export default [
+import { LexiconDoc } from '../../src/index'
+
+const lexicons: LexiconDoc[] = [
   {
     lexicon: 1,
     id: 'com.example.kitchenSink',
@@ -521,3 +523,5 @@ export default [
     },
   },
 ]
+
+export default lexicons
diff --git a/packages/lexicon/tests/general.test.ts b/packages/lexicon/tests/general.test.ts
@@ -1,5 +1,5 @@
 import { CID } from 'multiformats/cid'
-import { lexiconDoc, Lexicons } from '../src/index'
+import { LexiconDoc, Lexicons, parseLexiconDoc } from '../src/index'
 import LexiconDocs from './_scaffolds/lexicons'
 
 describe('Lexicons collection', () => {
@@ -97,7 +97,7 @@ describe('General validation', () => {
       },
     }
     expect(() => {
-      lexiconDoc.parse(schema)
+      parseLexiconDoc(schema)
     }).toThrow('Required field \\"foo\\" not defined')
   })
   it('fails when unknown fields are present', () => {
@@ -113,11 +113,11 @@ describe('General validation', () => {
     }
 
     expect(() => {
-      lexiconDoc.parse(schema)
+      parseLexiconDoc(schema)
     }).toThrow("Unrecognized key(s) in object: 'foo'")
   })
   it('fails lexicon parsing when uri is invalid', () => {
-    const schema = {
+    const schema: LexiconDoc = {
       lexicon: 1,
       id: 'com.example.invalidUri',
       defs: {
@@ -135,7 +135,7 @@ describe('General validation', () => {
     }).toThrow('Uri can only have one hash segment')
   })
   it('fails validation when ref uri has multiple hash segments', () => {
-    const schema = {
+    const schema: LexiconDoc = {
       lexicon: 1,
       id: 'com.example.invalidUri',
       defs: {
@@ -168,7 +168,7 @@ describe('General validation', () => {
     }).toThrow('Uri can only have one hash segment')
   })
   it('union handles both implicit and explicit #main', () => {
-    const schemas = [
+    const schemas: LexiconDoc[] = [
       {
         lexicon: 1,
         id: 'com.example.implicitMain',

diff --git a/packages/xrpc/src/client.ts b/packages/xrpc/src/client.ts
@@ -1,4 +1,4 @@
-import { Lexicons, ValidationError } from '@atproto/lexicon'
+import { LexiconDoc, Lexicons, ValidationError } from '@atproto/lexicon'
 import {
   getMethodSchemaHTTPMethod,
   constructMethodCallUri,
@@ -46,11 +46,11 @@ export class Client {
   // schemas
   // =
 
-  addLexicon(doc: unknown) {
+  addLexicon(doc: LexiconDoc) {
     this.lex.add(doc)
   }
 
-  addLexicons(docs: unknown[]) {
+  addLexicons(docs: LexiconDoc[]) {
     for (const doc of docs) {
       this.addLexicon(doc)
     }