feat(database): enable database multi-resource support #159
Conversation
src/database/core/RepoManager.ts
Outdated
| */ | ||
| private repos_: { | ||
| [name: string]: Repo; | ||
| [name: string]: { | ||
| [name: string]: Repo; |
There was a problem hiding this comment.
can you use something more specific than "name" for these? I think it's maybe appName and dbUrl or something ?
There was a problem hiding this comment.
Sure, no problem.
src/database/core/RepoManager.ts
Outdated
| for (const repo in this.repos_) { | ||
| this.repos_[repo].interrupt(); | ||
| for (const instance in this.repos_) { | ||
| for (const repo in this.repos_[instance]) { |
There was a problem hiding this comment.
Should these be:
"instance" => "appName"
"repo" => "dbUrl"
There was a problem hiding this comment.
Yup, changed this and the function below.
| if (repo) { | ||
| fatal('FIREBASE INTERNAL ERROR: Database initialized multiple times.'); | ||
| fatal( | ||
| 'Database initialized multiple times. Please make sure the format of the database URL matches with each database() call.' |
There was a problem hiding this comment.
Alternatively we could just return the existing instance. Would that cause other problems though? I guess app would end up with multiple references to it and might call INTERNAL.delete() multiple times, etc.?
I guess I don't really feel strongly. It's probably not too harmful to make them pass the exact same URL every time.
There was a problem hiding this comment.
Yeah, I don't know that I'm terribly heartbroken making users pass the extra string around, especially considering that they can still just use the project config instead.
This enables database to leverage multi instance feature enabling multiple database instances per app.