fix: review remarks

Author: CountRedClaw

src/apitypes/graphql/graphql.changes.ts

@@ -73,7 +73,6 @@ export const compareDocuments = async (
       metaKey: DIFF_META_KEY,
       originsFlag: ORIGINS_SYMBOL,
       diffsAggregatedFlag: DIFFS_AGGREGATED_META_KEY,
-      // mode: COMPARE_MODE_OPERATION,
       normalizedResult: true,
     },
   ) as { merged: GraphApiSchema; diffs: Diff[] }

src/apitypes/graphql/graphql.consts.ts

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 import { ResolvedVersionDocument, ZippableDocument } from '../../types'
-import { GraphQLDocumentType } from './graphql.types'
 
 export const GRAPHQL_API_TYPE = 'graphql' as const
 
@@ -47,5 +46,5 @@ export const GRAPHQL_TYPE = {
 } as const
 
 export function isGraphqlDocument(document: ZippableDocument | ResolvedVersionDocument): boolean {
-  return Object.values(GRAPHQL_DOCUMENT_TYPE).includes(document.type as GraphQLDocumentType)
+  return Object.values(GRAPHQL_DOCUMENT_TYPE).some(type => document.type === type)
 }

src/apitypes/rest/rest.changes.ts

@@ -108,7 +108,6 @@ export const compareDocuments = async (
       metaKey: DIFF_META_KEY,
       originsFlag: ORIGINS_SYMBOL,
       diffsAggregatedFlag: DIFFS_AGGREGATED_META_KEY,
-      // mode: COMPARE_MODE_OPERATION,
       normalizedResult: true,
     },
   ) as { merged: OpenAPIV3.Document; diffs: Diff[] }
@@ -123,8 +122,8 @@ export const compareDocuments = async (
   const tags = new Set<string>()
   const changedOperations: OperationChanges[] = []
 
-  for (const path of Object.keys((merged as OpenAPIV3.Document).paths)) {
-    const pathData = (merged as OpenAPIV3.Document).paths[path]
+  for (const path of Object.keys(merged.paths)) {
+    const pathData = merged.paths[path]
     if (typeof pathData !== 'object' || !pathData) { continue }
 
     for (const key of Object.keys(pathData)) {
@@ -179,7 +178,7 @@ export const compareDocuments = async (
     }
   }
 
-  return { operationChanges: changedOperations, tags: [...tags.values()] }
+  return { operationChanges: changedOperations, tags: Array.from(tags) }
 }
 
 async function reclassifyBreakingChanges(
@@ -207,10 +206,6 @@ async function reclassifyBreakingChanges(
   if (!previousOperation?.deprecatedItems) { return }
 
   for (const diff of onlyBreaking) {
-    if (diff.type !== breaking) {
-      continue
-    }
-
     const deprecatedInVersionsCount = previousOperation?.deprecatedInPreviousVersions?.length ?? 0
     if (isOperationRemove(diff) && deprecatedInVersionsCount > 1) {
       diff.type = risky

src/apitypes/rest/rest.utils.ts

@@ -100,7 +100,7 @@ export const extractRootServersDiffs = (doc: OpenAPIV3.Document): Diff[] => {
 export const extractRootSecurityDiffs = (doc: OpenAPIV3.Document): Diff[] => {
   const addedSecurityDiff = (doc as WithDiffMetaRecord<OpenAPIV3.Document>)[DIFF_META_KEY]?.security
   const securityDiffs = Object.values((doc.security as WithDiffMetaRecord<OpenAPIV3.SecurityRequirementObject[]>)?.[DIFF_META_KEY] ?? {})
-  const componentsSecuritySchemesDiffs = Object.values((doc.components?.securitySchemes as WithDiffMetaRecord<OpenAPIV3.ComponentsObject['securitySchemes']>)[DIFF_META_KEY] ?? {})
+  const componentsSecuritySchemesDiffs = Object.values((doc.components?.securitySchemes as WithDiffMetaRecord<Record<string, OpenAPIV3.SecuritySchemeObject>>)[DIFF_META_KEY] ?? {})
   return [
     ...(addedSecurityDiff ? [addedSecurityDiff] : []),
     ...securityDiffs,

src/builder.ts

@@ -19,6 +19,7 @@ import {
   BuildConfigBase,
   BuildConfigFile,
   BuildConfigRef,
+  BuilderType,
   ExportDocument,
   FileId,
   isPublishBuildConfig,
@@ -58,7 +59,6 @@ import {
   DEFAULT_BATCH_SIZE,
   DEFAULT_VALIDATION_RULES_SEVERITY_CONFIG,
   EXPORT_BUILD_TYPES,
-  ExportBuildType,
   MESSAGE_SEVERITY,
   SUPPORTED_FILE_FORMATS,
   VERSION_STATUS,
@@ -283,6 +283,22 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
     return this.buildResult
   }
 
+  private findApiBuilderByApiType(apiType: BuilderType): ApiBuilder {
+    const apiBuilder = this.apiBuilders.find(apiBuilder => apiBuilder.apiType === apiType)
+    if (!apiBuilder) {
+      throw new Error(`Cannot find apiBuilder for apiType ${apiType}`)
+    }
+    return apiBuilder
+  }
+
+  private findApiBuilderBySpecType(type: string): ApiBuilder {
+    const apiBuilder = this.apiBuilders.find(apiBuilder => apiBuilder.types.includes(type))
+    if (!apiBuilder) {
+      throw new Error(`Cannot find apiBuilder for specification type ${type}`)
+    }
+    return apiBuilder
+  }
+
   async parsedFileResolver(fileId: string): Promise<SourceFile | null> {
     if (this.parsedFiles.has(fileId)) {
       return this.parsedFiles.get(fileId) ?? null
@@ -325,11 +341,7 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
       if (!document) {
         throw new Error(`Raw document ${slug} is missing in local cache`)
       }
-      const apiBuilder = this.apiBuilders.find(apiBuilder => apiBuilder.types.includes(document?.type))
-      if (!apiBuilder) {
-        // todo
-        throw new Error(`Cannot find apiBuilder for type ${document?.type}`)
-      }
+      const apiBuilder = this.findApiBuilderBySpecType(document.type)
       return new File([apiBuilder.dumpDocument(document)], document.filename)
     }
 
@@ -475,12 +487,14 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
     apiType?: OperationsApiType,
   ): Promise<ResolvedVersionDocuments | null> {
     packageId = packageId ?? this.config.packageId
-    // this is the case when a version has been built just now, and there's nothing to fetch yet, so
-    // the only way to get the docs is to get them from buildResult, but the referenced packages map will be empty (packages: {})
     if (this.canBeResolvedLocally(version, packageId)) {
-      const apiBuilder = this.apiBuilders.find(apiBuilder => apiBuilder.apiType === apiType)
-      const currentApiTypeDocuments = this.documentList.filter(({ type }) => apiBuilder?.types.includes(type))
-      return { documents: currentApiTypeDocuments, packages: {} }
+      // this is the case when a version has been built just now, and there's nothing to fetch yet, so
+      // the only way to get the docs is to get them from buildResult, but the referenced packages map will be empty (packages: {})
+      if (apiType) {
+        const apiBuilder = this.findApiBuilderByApiType(apiType)
+        return { documents: this.documentList.filter(({ type }) => apiBuilder.types.includes(type)), packages: {} }
+      }
+      return { documents: this.documentList, packages: {} }
     }
 
     const { versionDocumentsResolver } = this.params.resolvers
@@ -538,7 +552,7 @@ export class PackageVersionBuilder implements IPackageVersionBuilder {
   private canBeResolvedLocally(version: string, packageId: string | undefined): boolean {
     return this.config.buildType !== BUILD_TYPE.CHANGELOG &&
       this.config.buildType !== BUILD_TYPE.PREFIX_GROUPS_CHANGELOG &&
-      !EXPORT_BUILD_TYPES.includes(this.config.buildType as ExportBuildType) &&
+      EXPORT_BUILD_TYPES.every(type => type !== this.config.buildType) &&
       version === this.config.version &&
       packageId === this.config.packageId
   }

src/components/compare/bwc.validation.ts

@@ -25,10 +25,9 @@ export function reclassifyNoBwcBreakingChanges(
 ): Diff[] {
   if (current?.apiKind === API_KIND.NO_BWC || previous?.apiKind === API_KIND.NO_BWC) {
     return operationDiffs?.map((diff) => {
-      if (diff.type !== BREAKING_CHANGE_TYPE) {
-        return diff
-      }
-      return { ...diff, type: RISKY_CHANGE_TYPE }
+      return diff.type === BREAKING_CHANGE_TYPE
+        ? { ...diff, type: RISKY_CHANGE_TYPE }
+        : diff
     })
   }
   return operationDiffs

src/components/compare/compare.utils.ts

@@ -89,29 +89,6 @@ export function getOperationTypesFromTwoVersions(
   return [prevVersion?.operationTypes || [], currVersion?.operationTypes || []]
 }
 
-type OperationIdWithoutGroupPrefix = string
-export type OperationIdentityMap = Record<OperationIdWithoutGroupPrefix, OperationId>
-
-export function dedupePairs<A extends object, B extends object>(
-  pairs: ReadonlyArray<[A, B]>,
-): Array<[A, B]> {
-  const seen = new WeakMap<A, WeakSet<B>>()
-  const out: Array<[A, B]> = []
-
-  for (const [a, b] of pairs) {
-    let bs = seen.get(a)
-    if (!bs) {
-      bs = new WeakSet<B>()
-      seen.set(a, bs)
-    }
-    if (bs.has(b)) continue
-    bs.add(b)
-    out.push([a, b])
-  }
-
-  return out
-}
-
 function dedupeTuples<T extends [object | undefined, object | undefined]>(
   tuples: T[],
 ): T[] {
@@ -138,7 +115,7 @@ function dedupeTuples<T extends [object | undefined, object | undefined]>(
   return result
 }
 
-export function normalizeOperationIds(operations: ResolvedOperation[], apiBuilder: ApiBuilder, groupSlug: string): [OperationId[], Record<NormalizedOperationId | OperationId, ResolvedOperation>] {
+export function normalizeOperationIds(operations: ResolvedOperation[], apiBuilder: ApiBuilder, groupSlug: string): [(NormalizedOperationId | OperationId)[], Record<NormalizedOperationId | OperationId, ResolvedOperation>] {
   const normalizedOperationIdToOperation: Record<NormalizedOperationId | OperationId, ResolvedOperation> = {}
   operations.forEach(operation => {
     const normalizedOperationId = apiBuilder.createNormalizedOperationId?.(operation) ?? operation.operationId
@@ -155,7 +132,6 @@ export function getOperationMetadata(operation: ResolvedOperation): OperationCha
 
     // api specific
     ...takeIfDefined({ method: operation.metadata?.method }),
-    // todo originalPath is missing?
     ...takeIfDefined({ path: operation.metadata?.path }),
     ...takeIfDefined({ type: operation.metadata?.type }),
   }

src/types/external/types.ts

@@ -30,5 +30,5 @@ export type TemplatePath = string
 export type VersionId = string | `${string}@${number}`
 export type OperationId = string
 
-export type WithDiffMetaRecord<T> = T & {[DIFF_META_KEY]?: DiffMetaRecord}
-export type WithAggregatedDiffs<T> = T & {[DIFFS_AGGREGATED_META_KEY]: Diff[]}
+export type WithDiffMetaRecord<T extends object> = T & {[DIFF_META_KEY]?: DiffMetaRecord}
+export type WithAggregatedDiffs<T extends object> = T & {[DIFFS_AGGREGATED_META_KEY]: Diff[]}