featherscloud / wings

MIT License
11 stars 0 forks source link

Feature Request - previousData #7

Open DaddyWarbucks opened 11 months ago

DaddyWarbucks commented 11 months ago

I love what you guys are doing here! I have often used these classes without actually integrating them into a feathers app. So I think this is a great idea.

Many of these classes do some initial lookup for their mutative methods. For example, https://github.com/wingshq/wings/blob/b991db0307e0ea08b7de29fe884b86018f9284ee/packages/knex/src/index.ts#L289 I have many instances where I am looking up the current record in order to do validation, side effects, etc.

const previousData = async (context) => {
  context.params.previousData = await service._get(context.id, context.params);
  return context;
}

Later hooks (both before and after) then use context.params.previousData to do all kinds of stuff. I have always wanted to be able to pass that param to the service method and skip its lookup.

// Knex update method
const oldData = params.previousData || await this.get(id, params)

There are some concerns when using the multi option. And there are also some concerns with whether the user passes the properly shaped previousData.

Any thoughts?

daffl commented 11 months ago

Thanks! I think this will be a good move forward because it'll hopefully also help to decouple Feathers services and database access. We could definitely clean up a few things when you don't have to concern yourself with external access.

I think the question here would be how previous data should behave across all instances. Not all databases require the previous data to e.g. do a patch (which seems to be much more commonly used than update).

DaddyWarbucks commented 11 months ago

That makes sense. I know there have been some discussions about fast/bulk patches as well, which would affect this concept. This is just something I have always noticed in most adapters and thought this was a good place to document it.

Another solution may be to handle it in _findOrGet.

async _findOrGet(id: NullableAdapterId, params?: Params) {
  if (params.previousData) {
    return params.previousData;
  }
  return id === null ? await this.find(params) : await this.get(id, params)
}