Skip to content

Add .setPriority() for updating priority of a queued promise function#209

Merged
sindresorhus merged 21 commits intosindresorhus:mainfrom
RaishavHanspal:feature/update-priority
Jan 22, 2025
Merged

Add .setPriority() for updating priority of a queued promise function#209
sindresorhus merged 21 commits intosindresorhus:mainfrom
RaishavHanspal:feature/update-priority

Conversation

@RaishavHanspal
Copy link
Copy Markdown
Contributor

@RaishavHanspal RaishavHanspal commented Jul 11, 2024

Fixes #208 - add a uid to track the promise functions and update the priority on any promise.
also add a new event started to notify when a promise function is executed.

@sindresorhus sindresorhus requested a review from Richienb July 17, 2024 01:45
@Richienb Richienb mentioned this pull request Jul 17, 2024
@RaishavHanspal
Copy link
Copy Markdown
Contributor Author

requested changes have been pushed! @Richienb

@Richienb
Copy link
Copy Markdown
Collaborator

Richienb commented Jul 27, 2024

I think we should allow searching by the object returned by enqueue as well. What do you think? Does there remain a use case where a string id remains more useful?

@RaishavHanspal
Copy link
Copy Markdown
Contributor Author

@Richienb that would be a very nice idea, and will not require id property. The problem that I faced was enqueue accepts RunFunction - which in my understanding is a callback not accessible to the end-user.
The Idea of priority update - is for the user to be able to update the priority on the fly, currently this is achieved by a unique id assigned by user, or this can be passed using the invoke event as added in #210 incase assigned by #idAssigner.

@RaishavHanspal
Copy link
Copy Markdown
Contributor Author

RaishavHanspal commented Aug 9, 2024

@Richienb, @sindresorhus , Please suggest further.

@sindresorhus
Copy link
Copy Markdown
Owner

Needs to be added to the readme.

I would like to see some more tests.

And the pull request needs a proper title.

@RaishavHanspal RaishavHanspal changed the title Feature/update priority Add functionality to update priority of a promise function on the fly Nov 20, 2024
source/index.ts Outdated
}

/**
Update priority of a known promise function, using the `id` identifier, and a priority value to override existing priority value. The updated value of priority ensures whether to execute this promise function sooner or later.
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.

Readme and index.d.ts should be in sync.

@sindresorhus
Copy link
Copy Markdown
Owner

index.d.ts and readme should be in sync, specifically setPriority

@RaishavHanspal
Copy link
Copy Markdown
Contributor Author

Ok @sindresorhus ! would you suggest I add the code snippets as well in the json doc comment just as we have in the readme? Or please suggest otherwise. 🙂

@sindresorhus
Copy link
Copy Markdown
Owner

Yes

@sindresorhus sindresorhus requested review from Richienb and removed request for Richienb December 4, 2024 01:40
Copy link
Copy Markdown
Collaborator

@Richienb Richienb left a comment

Choose a reason for hiding this comment

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

This is looking pretty good

@Richienb Richienb changed the title Add functionality to update priority of a promise function on the fly Add .setPriority() for updating priority of a queued promise function Jan 4, 2025
Copy link
Copy Markdown
Collaborator

@Richienb Richienb left a comment

Choose a reason for hiding this comment

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

(Despite #209 (comment))

I'm loving it

@sindresorhus sindresorhus merged commit 6e5cbc9 into sindresorhus:main Jan 22, 2025
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.

Is it feasible to change priority of a Queued item on the fly

3 participants