Skip to content

Query context provider - #2

Merged
Gabswim merged 8 commits into
Gabswim:feat/lit-query-examplefrom
mindroute:feat/lit-query-example
Sep 25, 2024
Merged

Query context provider#2
Gabswim merged 8 commits into
Gabswim:feat/lit-query-examplefrom
mindroute:feat/lit-query-example

Conversation

@klasjersevi

Copy link
Copy Markdown

Added a proper query context provider. To get it to work, some things in QueryController had to be changed to allow an asynchronous flow.

Changes:

  • Fixed build
  • Added context provider
  • Removed query client helper
  • Fixed and added tests

Comment thread packages/lit-query/package.json
Comment thread packages/lit-query/package.json
Comment thread packages/lit-query/src/__tests__/QueryController.test.ts
describe('pending', () => {
beforeEach(() => {
getTodoById.mockImplementationOnce(() => new Promise(() => {}))
//getTodoById.mockImplementationOnce(() => new Promise(() => {}))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should uncomment that? This is to simulate what we are loading. If not I would delete it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a bit of a problem here. I think there should be a fail-safe simulation here (like it was). But enabling this line makes all subsequent tests fail. Right now, the state happens to be "loading" when this test runs, so it passes. But this test is most likely not fail-safe this way.

I don't know enough about the mocking to make a fail-safe fix for this.

Comment thread packages/lit-query/src/QueryController.ts
Comment thread packages/lit-query/src/testHelpers.ts Outdated
Comment on lines +42 to +50
if (result?.isPending) {
return html`<div>Loading...</div>`
}

if (result.isError) {
if (result?.isError) {
return html`<div>Error</div>`
}

const { userId, id, title, completed } = result.data
const { userId, id, title, completed } = result?.data ?? {}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the best but I don't have any better idea on how to make it better because of the asynchronous nature of the lit-context. Maybe https://github.com/justinfagnani could help us with that.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, there are a number of ways to go here. Right now the result is initially undefined which is reasonable, but it makes it harder to use destructuring. Therefore the defaulting {}. Another way would be to return an empty result object initially.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start with that and we can always come back to it.

@Gabswim Gabswim left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will approve right after you address the comments. Thank you for your time! Great job!

Remove unused line

Co-authored-by: Gabriel Legault <gablegault1@hotmail.com>
@Gabswim
Gabswim self-requested a review September 25, 2024 03:03
@Gabswim
Gabswim merged commit 68e3c7e into Gabswim:feat/lit-query-example Sep 25, 2024
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

Successfully merging this pull request may close these issues.

2 participants