Skip to content

docs(docs-infra): replace wait with debounce for search#67897

Open
SkyZeroZx wants to merge 1 commit intoangular:mainfrom
SkyZeroZx:docs-infra-debounce
Open

docs(docs-infra): replace wait with debounce for search#67897
SkyZeroZx wants to merge 1 commit intoangular:mainfrom
SkyZeroZx:docs-infra-debounce

Conversation

@SkyZeroZx
Copy link
Contributor

@SkyZeroZx SkyZeroZx commented Mar 26, 2026

Replaces custom search debounce with debounced

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from josephperrott March 26, 2026 18:02
@angular-robot angular-robot bot added area: docs Related to the documentation area: docs-infra Angular.dev application and infrastructure labels Mar 26, 2026
@ngbot ngbot bot added this to the Backlog milestone Mar 26, 2026
@josephperrott josephperrott requested review from JeanMeche and removed request for josephperrott March 26, 2026 18:03
Copy link
Member

@JeanMeche JeanMeche left a comment

Choose a reason for hiding this comment

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

🚀

@JeanMeche JeanMeche added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Mar 26, 2026
Replaces custom search debounce with `debounced`
@SkyZeroZx SkyZeroZx force-pushed the docs-infra-debounce branch from 41d9dfd to 6038b73 Compare March 26, 2026 19:52
@angular-robot angular-robot bot requested a review from JeanMeche March 26, 2026 19:53

let search: Search;

useAutoTick();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to run the tests when I made the change.
I updated the test and had to copy some utilities we had in private/testing since I couldn't find an easy way to expose it in adev

Copy link
Member

Choose a reason for hiding this comment

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

Yeah better to copy what's not part of the public api

TestBed.inject(ApplicationRef).tick();

// The delay from debounced (200ms)
await timeout(300);
Copy link
Member

Choose a reason for hiding this comment

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

you shouldn't await if you use autotick

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC , it's still necessary; autoTick prevented time from accumulating (depending on how many setTimeouts we have, more would accumulate, which would be a waste of resources).

Copy link
Member

Choose a reason for hiding this comment

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

I would expect a jasmine.clock().tick(300) here.

Copy link
Contributor Author

@SkyZeroZx SkyZeroZx Mar 26, 2026

Choose a reason for hiding this comment

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

We can change it, but it would have to be the equivalent of using a microtask flush , otherwise we'll get an error

jasmine.clock().tick(300)
await Promise.resolve()

or using only await timeout(300)

Copy link
Member

Choose a reason for hiding this comment

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

My issue is that waiting 300ms is too long (as it can add up if we do this on multiple tests).
Having a await Promise.resolve() is fine as it doesn't wait for long.

Copy link
Contributor Author

@SkyZeroZx SkyZeroZx Mar 26, 2026

Choose a reason for hiding this comment

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

I understand, that’s exactly what autoTick is for. It avoids the main drawback of “waiting” and lets the timeout utility (which uses setTimeout under the hood) run synchronously

Using autoTick with timeout is equivalent to

jasmine.clock().tick(300)
await Promise.resolve()

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

Labels

action: merge The PR is ready for merge by the caretaker area: docs Related to the documentation area: docs-infra Angular.dev application and infrastructure target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants