feathersjs-ecosystem / feathers-sync

Synchronize service events between Feathers application instances
MIT License
222 stars 41 forks source link

feathers-sync couldnt understand mongodb+srv #112

Closed msudgh closed 5 years ago

msudgh commented 5 years ago

I've tried to pass this connection string to sync package but its unable to understand mongodb connection string with srv and i think splitting connection string without at least using indexOf isn't enough.

https://github.com/feathersjs-ecosystem/feathers-sync/blob/e95a86643f9b42dc6f32004feab011607b791dab/lib/index.js#L24

example of mongodb+srv:

mongodb+srv://test:test@mongodb.net

result of these line codes:

const name = protocol.substring(0, protocol.length - 1); // 'mongodb+srv'
const adapter = mappings[name]; // undefined
daffl commented 5 years ago

If the protocol is not enough to determine the adapter you can use the alternative initialization API documented here:

// Configure MongoDB with an existing connection
app.configure(sync.mongodb({
  db: config.uri,
  collection: 'events'
}));
msudgh commented 5 years ago

when we're designing a generic API it must have a bare configuration and not an advanced model. the current approach is mixed 🤦‍♂️.

daffl commented 5 years ago

A generic API needs to cover the 90% case in a generic way but still allow more advanced use cases which is what the adapter based approach of the design of this module is doing. Either way, this has been addressed now, thank you for the PR 😄