Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify connection config via an object and not string #10339

Open
ad-si opened this issue Jun 9, 2021 · 14 comments
Open

Specify connection config via an object and not string #10339

ad-si opened this issue Jun 9, 2021 · 14 comments
Labels
new feature This change adds new functionality, like a new method or class

Comments

@ad-si
Copy link

ad-si commented Jun 9, 2021

Sorry if I'm blind, but how can I specify the connection with an object and not a string? E.g.

mongoose.connect({
  username: 'user123',
  password: 'pass123',
  host: 'mongodb0.example.com',
  port: 1234,
})
@IslandRhythms IslandRhythms added the help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary label Jun 9, 2021
@IslandRhythms
Copy link
Collaborator

Do you mean how is it possible to not use a string or specify the connection string with an object instead of a string?

@ad-si
Copy link
Author

ad-si commented Jun 9, 2021

Do you mean how is it possible to not use a string or specify the connection string with an object instead of a string?

Yeah, pretty much exactly like in my example

@IslandRhythms
Copy link
Collaborator

IslandRhythms commented Jun 9, 2021

So there is no native way to do this with mongoose. You could save each part as a variable and concatenate them to achieve a similar effect.

@ad-si
Copy link
Author

ad-si commented Jun 9, 2021

😳 Wow, I'm baffled that this is not supported yet.
Well then this is a feature request for it. An Object is

  1. much easier to handle
  2. Can give much better error messages in combination with TypeScript
  3. Is the JS default for configuring anything

@IslandRhythms IslandRhythms added new feature This change adds new functionality, like a new method or class and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary labels Jun 9, 2021
@ShadiestGoat
Copy link
Contributor

I mean to be fair though, this isn't that hard to implement. My current setup is just get the config, and generate a link using a config

const conf:{
    mongodb: {
        username: string,
        password: string,
        ip: string,
        db: string,
        port: string
    }
} = configFunction()
const MongoDBendPoint = "mongodb://" + conf.mongodb.username + ":" + conf.mongodb.password + "@" + conf.mongodb.ip + ":" + conf.mongodb.port + "/" + conf.mongodb.db + "?readPreference=primary&appname=MyAppNameHere&ssl=false?authSource=" + conf.mongodb.db
//or
const MongoDBendPoint2 = `mongodb://${conf.mongodb.username}:${conf.mongodb.password}@${conf.mongodb.ip}:${conf.mongodb.port}/${conf.mongodb.db}?readPreference=primary&appname=MyAppNameHere&ssl=false?authSource=${conf.mongodb.db}`

@ad-si
Copy link
Author

ad-si commented Jun 13, 2021

  1. Did you handle URL encoding correctly?
  2. A lot of invalid values for the parameters could generate a potentially valid MongoDB string. Such things are quite error prone and hard to debug.
  3. This code is exceptionally convoluted and ugly. More potential for mistakes. (Would be too long for the common 80 chars limit per line anyways)

What not simply add the function to build this string to mongoose? Including type / range checking, normalization, and so on.

@ShadiestGoat
Copy link
Contributor

I'm not saying it's a perfect solution, I'm just saying that it could be a workaround-ish solution, for now. I'm very not against having this function be in mongoose

@vkarpov15
Copy link
Collaborator

Not a bad idea. The only potential downside is that connection string is MongoDB's default approach, so makes it harder to copy/paste connection string from Atlas or Studio 3T. But worth considering for the future.

@vkarpov15 vkarpov15 added this to the 5.x Unprioritized milestone Jun 14, 2021
@ShadiestGoat
Copy link
Contributor

Wait, are you suggesting to replace it completely?

@ad-si
Copy link
Author

ad-si commented Jun 15, 2021

Well I don't 😳

@vkarpov15
Copy link
Collaborator

@ShadiestGoat not suggesting replacing it completely, that would be too big a breaking change for too little benefit IMO. But a named parameters style syntax for connect() might be a good new feature, especially given that MongoDB expects you to escape username/password before putting them into connection strings.

@ShadiestGoat
Copy link
Contributor

Yeah, I no sorry I was a bit worried that you were going to replace it completely, I don't see any benefits to that personally. The connect should be string | loginParams

@ad-si
Copy link
Author

ad-si commented Jun 18, 2021

Actually, on second thought, I think it would be cleaner to have two functions:

mongoose.connect :: string -> Connection
mongoose.connectConfig :: loginParams -> Connection

@vkarpov15
Copy link
Collaborator

Sure, mongoose.connectWithConfig() and Connection#openWithConfig() sounds like a reasonable alternative to avoid too many different overrides for mongoose.connect().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

4 participants