feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
14.98k stars 743 forks source link

Docs: MongoDB teardown docs have non-working code, due to bad generated code #3203

Closed FossPrime closed 1 year ago

FossPrime commented 1 year ago

Steps to reproduce

The teardown docs include a line about MongoDB which will not work with the default generated code.

await context.app.get('mongoClient').close()

That cannot work with the default generated mongoClient app global:

export const mongodb = (app: Application) => {
  const connection = app.get('mongodb') as string
  const database = new URL(connection).pathname.substring(1)
  const mongoClient = MongoClient.connect(connection).then((client) => client.db(database))

  app.set('mongodbClient', mongoClient)
}

app.get('mongodbClient') is of type Db, not MongoClient. Db does not have a close method, MongoClient does; but Feathers' mongodbClient is not of type MongoClient. app.get('mongoClient') is not something available in default installations.

Proposed solution

  1. Move model creation to mongodb.js. Remove mongodbClient app global. Export a teardown hook, or set in place.

OR

  1. Add mongoClient and set mongoClient to MongoClient.connect(connection).
// For more information about this file see https://dove.feathersjs.com/guides/cli/databases.html
import { MongoClient } from 'mongodb'
import type { Db } from 'mongodb'
import type { Application } from './declarations'

declare module './declarations' {
  interface Configuration {
    mongoClient: Promise<MongoClient>
    mongodbClient: Promise<Db>
  }
}

export const mongodb = (app: Application) => {
  const connection = app.get('mongodb') as string
  const database = new URL(connection).pathname.substring(1)
  const mongoClient = MongoClient.connect(connection)
  const mongodbClient = mongoClient.then((client) => client.db(database))

  app.set('mongoClient', mongoClient)
  app.set('mongodbClient', mongodbClient)
}
FossPrime commented 1 year ago

The MongoDB adapter needs a lot of work... like a lot. This is not high priority among them.