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

Angular persist query client experimental #8324

Open
wants to merge 39 commits into
base: main
Choose a base branch
from

Conversation

OmerGronich
Copy link

@OmerGronich OmerGronich commented Nov 22, 2024

Angular Query Persister Plugin

  • Created angular-query-persist-client-package
  • Implemented withPersistQueryClient in new package
  • Added isRestoring provider and injection token to main angular-query package

@OmerGronich OmerGronich marked this pull request as draft November 22, 2024 14:58
@OmerGronich OmerGronich force-pushed the angular-persist-query-client-experimental branch from 8474149 to 31c8c0e Compare November 22, 2024 15:01
Copy link

nx-cloud bot commented Nov 22, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5d80309. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx affected --targets=test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build --parallel=3
✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@OmerGronich OmerGronich force-pushed the angular-persist-query-client-experimental branch 3 times, most recently from 0ff5f5c to b4d880a Compare November 23, 2024 12:00
@OmerGronich OmerGronich deleted the angular-persist-query-client-experimental branch November 23, 2024 12:00
@OmerGronich OmerGronich restored the angular-persist-query-client-experimental branch November 23, 2024 12:00
@OmerGronich OmerGronich reopened this Nov 23, 2024
@OmerGronich OmerGronich force-pushed the angular-persist-query-client-experimental branch from 566f72a to e1ae58d Compare November 23, 2024 19:42
@OmerGronich OmerGronich marked this pull request as ready for review November 23, 2024 20:12
@arnoud-dv arnoud-dv self-assigned this Nov 26, 2024
@OmerGronich OmerGronich force-pushed the angular-persist-query-client-experimental branch 2 times, most recently from d4fa95c to 9e4182a Compare November 29, 2024 13:27
Copy link
Collaborator

@arnoud-dv arnoud-dv left a comment

Choose a reason for hiding this comment

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

Some suggestions for now, I still need to do a more in-depth review of the persistence itself.

@OmerGronich OmerGronich force-pushed the angular-persist-query-client-experimental branch 3 times, most recently from 3c3c8ec to ed0ed3c Compare December 7, 2024 14:40
@OmerGronich OmerGronich force-pushed the angular-persist-query-client-experimental branch from ed0ed3c to 325b2a7 Compare December 7, 2024 20:44
Copy link
Member

@crutchcorn crutchcorn left a comment

Choose a reason for hiding this comment

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

I was requested to review this PR from OP - it generally looks good to me! Seems like a lot of edgecases handled as well.

Anything you want to me to take a closer eye on?

@@ -104,33 +106,33 @@ export function createBaseQuery<
},
)

effect(() => {
effect((onCleanup) => {
Copy link
Author

@OmerGronich OmerGronich Mar 12, 2025

Choose a reason for hiding this comment

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

I was requested to review this PR from OP - it generally looks good to me! Seems like a lot of edgecases handled as well.

Anything you want to me to take a closer eye on?

@crutchcorn Yes, one thing I wanted an opinion on is this cleanup that I added here, does it make sense to you? Do we need to do the same in the injectQueries function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants