Should I refrain from circular imports when using typescript?-CodePudding

I just spent 4 hours troubleshooting typescript error

Object literal may only specify known properties, and 'details'
does not exist in type 'Readonly<{ [x: `details.${string}.value`]:
{ value: string; type: string; } | undefined; [x: `details.${string}.type`]:
{ value: string; type: string; } | undefined;

and clearly, I can see there is 'details' in the object whatever tsc is complaining on. Literally, I have other functions on the same file working without typescript errors.

Long story short, Controller will invoke Service with a single parameter and its type is defined on Controller file but then Service needs to know what type of data its getting so Service file has import statement pointing to Controller

Controller

import service from './Service'
export type DataType = {...}

const data:DataType = fetch()
service(data)

Service

import DataType from './controller'

export function service(data:DataType) {
  // typescript error
  someOtherFunctionCall<DataType>(data)
}

I think it's a pretty basic pattern. When I deleted import statement from service file, the error went away and of course, copy/pasted type definition. Weird thing is that error didn't show up again when I pasted exactly the same line. Is it expected behavior? I've heard nodejs can handle not very complicated circular imports. But this happened, should I not use circular imports?

Below are code from my project

Category.ts

export default interface Category {
  _id: ObjectId
  details: Record<
    string,
    {
      value: string
      type: string
    }
  >
  ...
}

controller.ts

import { updateItem } from './service'

export interface ScrapItem {
  _id: ObjectId
  details: Record<
    string,
    {
      value: string
      type: string
    }
  >
  ...
}

const item:ScrapItem = scrapSomeItem()
updateItem(item)

service.ts

import Category from 'models/category'
import ScrapItem from './controller'
import db from 'MongodbHelper'

export async function updateItem(item: ScrapItem): Promise<void> {
  const db = await getDb()
  const categoryCollection = db.collection<Category>(category)

  await categoryCollection.updateOne(
    {
      _id
    },
    {
      $set: {
        // THIS IS WHERE I GET ERROR
        // Object literal may only specify known properties, and 'details' does not exist in type ...
        details,
        ...
      }
    },
    {
      upsert: true
    }
  )
}

I'm getting typescript error on service.ts where I call one of mongodb driver method updateOne. The only workaround here is to delete Category from const categoryCollection = db.collection<Category>(category) or add | any to details type definition on category.ts.

CodePudding user response：

This is actually a bug in the mongodb version 4.8 typing declaration files. It has been fixed already on the main branch 3 days ago (2022-07-20) but there is no release yet. The latest release, the 4.8 version at the time of this writing, is from 10 days ago (2022-07-13).

To fix the problem downgrade the version to 4.7:

npm i mongodb@~4.7.0

Or install from the main branch where the fix is already inplace:

npm i "https://github.com/mongodb/node-mongodb-native#05e007b0b5ff98151c3ff972ee2f881b4203639e"

CodePudding user response：

Circular imports are considered a bad practice. It makes the architecture of a software system unnecessarily complex and obscure. Furthermore, it affects build time and the overall testability of the components that you are building.

Not entirely related to Typescript, but after a proposal for allowing import cycles in Golang, Robert Pike (one of the creators of Go) responded:

The lack of import cycles in Go forces programmers to think more about their dependencies and keep the dependency graph clean and builds fast. Conversely, allowing cycles enables laziness, poor dependency management, and slow builds. Eventually one ends up with a single cyclical blob enclosing the entire dependency graph and forcing it into a single build object. This is very bad for build performance and dependency resolution. These blobs are also take much more work to detangle than the is required to keep the graph a proper DAG in the first place. This is one area where up-front simplicity is worthwhile. Import cycles can be convenient but their cost can be catastrophic. They should continue to be disallowed.

(Source: https://github.com/golang/go/issues/30247#issuecomment-463940936)

I don't know how much of the compile issues in Go affect Typescript, but I know for sure that on the long run it's better to avoid circular imports for the sake of clarity and good design. And also if you are getting errors because of it.