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

Add size() to expose amount of requests in queue #2941

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

StephanBijzitter
Copy link
Contributor

R: @jeffposnick @tropicadri

Fixes #2940

In short, this allows a developer to perform a quick count of what's left in the queue, so that a number could be displayed in a status indicator... or whatever else the developer wants to do with the number.

It's not as heavy as (await queue.getAll()).length and doesn't modify the queue

  • ensure that gulp build && gulp lint test passes locally.

Well, I tried... but Windows doesn't very much like the npm scripts and as such I had to bypass the git hooks as well, it just wouldn't run.. Hopefully I managed to keep everything consistent with the surrounding code :-)

@jeffposnick
Copy link
Contributor

Thanks for submitting this!

We have a lot of workbox-background-sync feature requests open at the moment, and I have to admit that it's one of the areas of the Workbox codebase that I am personally less familiar with. I'm going to take some time to look at this change and see whether it also address some of the other open issues.

(Regarding running the gulp tasks, apologies about that. I'll take a look at our new setup and figure out the Windows compatibility story.)

Copy link
Contributor

@jeffposnick jeffposnick left a comment

Choose a reason for hiding this comment

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

I'd like @tropicadri to get a chance to review this, as she's touched the IndexedDB code most recently, but generally speaking, this looks like it would be useful as implemented.

test/workbox-background-sync/sw/lib/test-QueueDb.mjs Outdated Show resolved Hide resolved
Note that expired entries (per `maxRetentionTime`) are also included in this count

Closes GoogleChrome#2940
Copy link
Collaborator

@tropicadri tropicadri left a comment

Choose a reason for hiding this comment

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

LGTM Thank you!

@tropicadri
Copy link
Collaborator

I'd like @tropicadri to get a chance to review this, as she's touched the IndexedDB code most recently, but generally speaking, this looks like it would be useful as implemented.

I checked it out, I agree with you :)

@jeffposnick jeffposnick self-requested a review September 29, 2021 18:37
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.

expose amount of requests in queue
3 participants