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

feat(connect): support promise for connectionString #41

Merged
merged 2 commits into from
Dec 16, 2019

Conversation

skeggse
Copy link
Member

@skeggse skeggse commented Dec 13, 2019

There are cases where it's useful to have a synchronous reference to the Database, and where the connection information is not yet available - particularly in test cases where you may be launching a temporary mongo database using mongo-memory-server. To better support this use-case, the Database class can accept and store the connectionString parameter as a Promise, and resolve it to its value in the connect handler which is already well-positioned to wait for asynchronous resolution.

Copy link
Member

@ghmeier ghmeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough examples! Overall this seems like useful, non-intrusive functionality.

lib/database.js Show resolved Hide resolved
Eli Skeggs added 2 commits December 16, 2019 10:33
There are cases where it's useful to have a synchronous reference to the Database, and where the
connection information is not yet available - particularly in test cases where you may be launching
a temporary mongo database using mongo-memory-server. To better support this use-case, the Database
class can accept and store the connectionString parameter as a Promise, and resolve it to its value
in the connect handler which is already well-positioned to wait for asynchronous resolution.
@skeggse skeggse merged commit 71b1e61 into master Dec 16, 2019
@skeggse skeggse deleted the support-promise-connection branch December 16, 2019 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants