Merge pull request #132 from lishaduck/extraneous-fields

Fix addition of extraneous fields, closes #130
jeffijoe · Nov 11, 2024 · e03f65a · e03f65a
2 parents fee1f00 + b3e3072
commit e03f65a
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 47 deletions.
diff --git a/src/__tests__/type-syncer.test.ts b/src/__tests__/type-syncer.test.ts
@@ -126,12 +126,17 @@ function buildSyncer() {
   }
 
   const package2File: ITestPackageFile = {
-    name: 'package-1',
+    name: 'package-2',
     dependencies: {
       package3: '^1.0.0',
     },
   }
 
+  const package3File: ITestPackageFile = {
+    name: 'package-3',
+    dependencies: {},
+  }
+
   const packageService: IPackageJSONService = {
     readPackageFile: jest.fn(async (filepath: string) => {
       switch (filepath) {
@@ -145,6 +150,8 @@ function buildSyncer() {
           return package1File
         case 'packages/package-2/package.json':
           return package2File
+        case 'packages/package-3/package.json':
+          return package3File
         default:
           throw new Error(`Who?! ${filepath}`)
       }
@@ -161,20 +168,11 @@ function buildSyncer() {
           workspaces = rootPackageFile.workspaces
           break
         }
-        case 'packages/package-1/': {
-          workspaces = package1File.workspaces
-          break
-        }
-        case 'packages/package-2/': {
-          workspaces = package2File.workspaces
-          break
-        }
         default:
           throw new Error('What?!')
       }
 
-      workspaces ??= []
-      const globPromises = workspaces.map((w) =>
+      const globPromises = workspaces!.map((w) =>
         globber.glob(w, 'package.json'),
       )
       const globbed = await Promise.all(globPromises)
@@ -187,7 +185,11 @@ function buildSyncer() {
     glob: jest.fn(async (pattern, _filename) => {
       switch (pattern) {
         case 'packages/*':
-          return ['packages/package-1/', 'packages/package-2/']
+          return [
+            'packages/package-1/',
+            'packages/package-2/',
+            'packages/package-3/',
+          ]
         default:
           return []
       }
@@ -254,13 +256,19 @@ function buildSyncer() {
   }
 }
 
+type WritePackageFileMock = jest.Mock<
+  Promise<void>,
+  [filePath: string, fileContents: IPackageFile],
+  never
+>
+
 describe('type syncer', () => {
   it('adds new packages to package.json', async () => {
     const { syncer, packageService } = buildSyncer()
     const result = await syncer.sync('package.json', {})
     const writtenPackage = (
-      packageService.writePackageFile as jest.Mock<any>
-    ).mock.calls.find((c) => c[0] === 'package.json')[1] as IPackageFile
+      packageService.writePackageFile as WritePackageFileMock
+    ).mock.calls.find((c) => c[0] === 'package.json')![1] as IPackageFile
     expect(writtenPackage.devDependencies).toEqual({
       '@types/package1': '~1.0.0',
       '@types/package3': '~1.0.0',
@@ -274,7 +282,7 @@ describe('type syncer', () => {
       package5: '^1.0.0',
     })
 
-    expect(result.syncedFiles).toHaveLength(3)
+    expect(result.syncedFiles).toHaveLength(4)
 
     expect(result.syncedFiles[0].filePath).toEqual('package.json')
     expect(
@@ -302,16 +310,19 @@ describe('type syncer', () => {
     expect(
       result.syncedFiles[2].newTypings.map((x) => x.typingsName).sort(),
     ).toEqual(['package3'])
+    expect(result.syncedFiles[3].package.devDependencies).toStrictEqual(
+      undefined,
+    )
   })
 
   it('ignores deps when asked to', async () => {
     const { syncer, packageService } = buildSyncer()
     await syncer.sync('package-ignore-dev.json', {})
     const writtenPackage = (
-      packageService.writePackageFile as jest.Mock<any>
+      packageService.writePackageFile as WritePackageFileMock
     ).mock.calls.find(
       (c) => c[0] === 'package-ignore-dev.json',
-    )[1] as IPackageFile
+    )![1] as IPackageFile
     expect(writtenPackage.devDependencies).toEqual({
       '@types/package1': '~1.0.0',
       '@types/package3': '~1.0.0',
@@ -332,10 +343,11 @@ describe('type syncer', () => {
     const { syncer, packageService } = buildSyncer()
     await syncer.sync('package-ignore-package1.json', {})
     const writtenPackage = (
-      packageService.writePackageFile as jest.Mock<any>
+      packageService.writePackageFile as WritePackageFileMock
     ).mock.calls.find(
       (c) => c[0] === 'package-ignore-package1.json',
-    )[1] as IPackageFile
+    )![1] as IPackageFile
+
     expect(writtenPackage.devDependencies).toEqual({
       '@types/package3': '~1.0.0',
       '@types/package4': '^1.0.0',
@@ -353,7 +365,7 @@ describe('type syncer', () => {
     const { syncer, packageService } = buildSyncer()
     await syncer.sync('package.json', { dry: true })
     expect(
-      packageService.writePackageFile as jest.Mock<any>,
+      packageService.writePackageFile as WritePackageFileMock,
     ).not.toHaveBeenCalled()
   })
 

diff --git a/src/cli.ts b/src/cli.ts
@@ -64,16 +64,12 @@ async function run(syncer: ITypeSyncer) {
   const syncedFilesOutput = result.syncedFiles
     .map(renderSyncedFile)
     .join('\n\n')
-  const totals = result.syncedFiles
-    .map((f) => ({
-      newTypings: f.newTypings.length,
-    }))
-    .reduce(
-      (accum, next) => ({
-        newTypings: accum.newTypings + next.newTypings,
-      }),
-      { newTypings: 0 },
-    )
+  const totals = result.syncedFiles.reduce(
+    (accum, f) => ({
+      newTypings: accum.newTypings + f.newTypings.length,
+    }),
+    { newTypings: 0 },
+  )
 
   const syncMessage = chalk`\n\n${syncedFilesOutput}\n\n✨  Run {green typesync} again without the {gray --dry} flag to update your {gray package.json}.`
   if (flags.dry === 'fail' && totals.newTypings > 0) {

diff --git a/src/type-syncer.ts b/src/type-syncer.ts
@@ -65,7 +65,14 @@ export function createTypeSyncer(
 
     const syncedFiles: Array<ISyncedFile> = await Promise.all([
       syncFile(filePath, file, syncOpts, dryRun),
-      ...subManifests.map((p) => syncFile(p, null, syncOpts, dryRun)),
+      ...subManifests.map(async (p) =>
+        syncFile(
+          p,
+          await packageJSONService.readPackageFile(p),
+          syncOpts,
+          dryRun,
+        ),
+      ),
     ])
 
     return {
@@ -109,17 +116,17 @@ export function createTypeSyncer(
    */
   async function syncFile(
     filePath: string,
-    file: IPackageFile | null,
+    file: IPackageFile,
     opts: ISyncOptions,
     dryRun: boolean,
   ): Promise<ISyncedFile> {
     const { ignoreDeps, ignorePackages } = opts
 
-    const packageFile =
-      file ?? (await packageJSONService.readPackageFile(filePath))
     const allLocalPackages = Object.values(IDependencySection)
       .map((dep) => {
-        const section = getDependenciesBySection(packageFile, dep)
+        const section = getDependenciesBySection(file, dep)
+        if (!section) return []
+
         const ignoredSection = ignoreDeps?.includes(dep)
         return getPackagesFromSection(section, ignoredSection, ignorePackages)
       })
@@ -179,21 +186,24 @@ export function createTypeSyncer(
         }
       }),
     ).then(mergeObjects)
-    const devDeps = packageFile.devDependencies
+    const devDeps = file.devDependencies
     if (!dryRun) {
-      await packageJSONService.writePackageFile(filePath, {
-        ...packageFile,
-        devDependencies: orderObject({
+      const newPackageFile: IPackageFile = { ...file }
+
+      if (Object.keys(devDepsToAdd).length > 0) {
+        newPackageFile.devDependencies = orderObject({
           ...devDepsToAdd,
           ...devDeps,
-        }),
-      } as IPackageFile)
+        })
+      }
+
+      await packageJSONService.writePackageFile(filePath, newPackageFile)
     }
 
     return {
       filePath,
       newTypings: used,
-      package: packageFile,
+      package: file,
     }
   }
 }
@@ -298,7 +308,7 @@ function getPackagesFromSection(
 function getDependenciesBySection(
   file: IPackageFile,
   section: IDependencySection,
-): IDependenciesSection {
+): IDependenciesSection | undefined {
   const dependenciesSection = (() => {
     switch (section) {
       case IDependencySection.deps:
@@ -311,5 +321,5 @@ function getDependenciesBySection(
         return file.peerDependencies
     }
   })()
-  return dependenciesSection ?? {}
+  return dependenciesSection
 }
diff --git a/src/util.ts b/src/util.ts
@@ -35,11 +35,11 @@ export function filterMap<T, R>(
  *
  * @param source
  */
-export function shrinkObject<T extends object>(source: T): Required<T> {
-  const object: any = {}
+export function shrinkObject<T extends object>(source: T): Partial<T> {
+  const object: Partial<T> = {}
 
   for (const key in source) {
-    if (typeof source[key] !== 'undefined') {
+    if (source[key] !== undefined) {
       object[key] = source[key]
     }
   }