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

IndexedStoremeta.added property #107

Open
nichoth opened this issue Mar 4, 2024 · 5 comments
Open

IndexedStoremeta.added property #107

nichoth opened this issue Mar 4, 2024 · 5 comments

Comments

@nichoth
Copy link
Contributor

nichoth commented Mar 4, 2024

I am calling client.log.add in the browser code, and the returned metadata does not have the added field.

const client = new CrossTabClient({
    server: (import.meta.env.DEV ?
        'ws://localhost:8765' :
        'wss://logux.example.com'),
    store: new IndexedStore('try-logux'),
    subprotocol: '1.0.0',
    userId: 'anonymous',  // TODO: We will fill it, in Authentication recipe
    token: '123'  // TODO: We will fill it in Authentication recipe
})

// ...

const inc = increment()
const meta = await client.log.add(inc, { sync: true })  // should add `added` here
debug('the increment meta', meta) // <-- does not have 'added' prop

Looking at the source code, we see that 'added' is added to meta. I'm not sure why I don't see it.

@nichoth
Copy link
Contributor Author

nichoth commented Mar 4, 2024

I created a test that demonstrates my expectation:

https://github.com/nichoth/logux-client/blob/added/client/index.test.ts#L413

@ai
Copy link
Member

ai commented Mar 5, 2024

Strange.

  1. Can you send PR with this test to https://github.com/logux/core/blob/main/each-store-check/index.js
  2. Could you try this test after you replace this lines https://github.com/logux/client/blob/main/indexed-store/index.js#L55-L56 to
    if (exist) {
      return false
    } else {
      let added = await promisify(store.os('log', 'write').add(entry))
      delete store.adding[entry.created]
-     meta.added = added
-     return meta
+    return { ...meta, added }
    }
  }

nichoth added a commit to nichoth/logux-client that referenced this issue Mar 6, 2024
@nichoth
Copy link
Contributor Author

nichoth commented Mar 6, 2024

I tried changing the file in client/indexed-store as described — https://github.com/nichoth/logux-client/blob/added/indexed-store/index.js#L55 — but still the same test results. Also, on main branch in @logux/core, the tests do not pass for me.

pnpm test
image

But, it looks like there is already a check for 'added' there — https://github.com/logux/core/blob/main/each-store-check/index.js#L132

I'm not sure where it is failing.

@ai
Copy link
Member

ai commented Mar 6, 2024

That test checks that store keep added in storage. My theory that IndexedDB freeze object and add just doesn't return the right object.

@ai
Copy link
Member

ai commented Mar 7, 2024

I will look on next week (too busy because of the conference on this week)

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

No branches or pull requests

2 participants