Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

SearchPager: Implement queueing events until a listener appears #723

Merged
merged 3 commits into from
May 27, 2021

Conversation

seewer
Copy link
Contributor

@seewer seewer commented May 7, 2021

This fixes the same problems for paged searches as does
#610 for unpaged searches.

By passing an EventEmitter via callback there exist cases when events
are emitted before listeners are registered, resulting in missed
events.

The Change turns SearchPager into a CorkedEmitter which is already
used as a solution for non paged searches. Doing so requires the
internal 'search' event to be dropped.

This change adapts a test case originally by László Szűcs (@ifroz).

Signed-off-by: Philippe Seewer philippe.seewer@bfh.ch

This fixes the same problems for paged searches as does
ldapjs#610 for unpaged searches.

By passing an EventEmitter via callback there exist cases when events
are emitted before listeners are registered, resulting in missed
events.

The Change turns SearchPager into a CorkedEmitter which is already
used as a solution for non paged searches. Doing so requires the
internal 'search' event to be dropped.

This change adapts a test case originally by László Szűcs (@ifroz).

Signed-off-by: Philippe Seewer <philippe.seewer@bfh.ch>
@UziTech
Copy link
Member

UziTech commented May 7, 2021

Can you run npm run lint to fix the linting errors?

Signed-off-by: Philippe Seewer <philippe.seewer@bfh.ch>
@jsumners
Copy link
Member

jsumners commented May 9, 2021

Looks like this PR branch is not up-to-date with the target master branch. Please rebase.

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@UziTech UziTech merged commit f719574 into ldapjs:master May 27, 2021
@UziTech UziTech mentioned this pull request May 27, 2021
@jsumners
Copy link
Member

⚠️ This issue has been locked due to age. If you have encountered a recent
problem that seems to be covered by this issue, please open a new issue.

Please include a minimal reproducible example
when opening a new issue.

@ldapjs ldapjs locked as resolved and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants