logux / client

Logux base components to build web client
https://logux.org/
MIT License
656 stars 46 forks source link

Filter works incorrectly if fields are splitted by several log actions #115

Open LexxFedoroff opened 1 month ago

LexxFedoroff commented 1 month ago

Hello, I've debugged this issue https://github.com/logux/examples/issues/21 and found that there is a bug in the filters. I can reproduce it with a simple test:

it('filter works correctly if sent not all fields', async () => {
  let client = new TestClient('10')
  await client.connect()
  client.log.keepActions()

  let posts = createFilter(client, Post, { authorId: '1'})
  let unbind = posts.listen(() => {})
  await allTasks()

  await client.server.sendAll({ channel: 'posts/1', type: 'logux/subscribed' })
  await client.server.sendAll({
    fields: { title: 'A' },
    id: '1',
    type: 'posts/created'
  })
  await client.server.sendAll({
    fields: { authorId: '1' },
    id: '1',
    type: 'posts/changed'
  })
  await allTasks()

  expect(ensureLoaded(posts.get()).list).toEqual([
    { id: '1', isLoading: false, title: 'A', authorId: '1' },
  ])
  unbind()
})

I want to fix it but I need some information on how that should work. As I understand we need to accumulate log actions for an object until we receive the fields required for a filter. What is the best way to achieve this? Version: 0.21.1

ai commented 1 month ago

Hm. I thought about this case.

This block should do the stuff https://github.com/logux/client/blob/main/create-filter/index.js#L280-L291

Can you investigate why it doesn’t work?

LexxFedoroff commented 1 month ago

I've investigated and I think the problem comes from the commit https://github.com/logux/client/commit/e2dac178fc7b62b9fde8fdb27f4764cd4c54883b Currently, the sync map is created from the last received action but this action doesn't contain all necessary fields. @euaaaio what do you think?

euaaaio commented 1 month ago

I won't be able to review it closely today.

I only remember that we had a problem with the implementation, which was somehow very different from how it was designed. Could it be that the bug is in the example repository?

LexxFedoroff commented 1 month ago

Could it be that the bug is in the example repository?

not sure, I think the bug is in the createFilter method I'm a newbie in the logux framework and I don't know exactly how it should work but I see two options:

the second option was implemented before the commit https://github.com/logux/client/commit/e2dac178fc7b62b9fde8fdb27f4764cd4c54883b and I tend that was correct