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

feat: support setting timeout for client:idle #11743

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

ph1p
Copy link
Contributor

@ph1p ph1p commented Aug 16, 2024

Changes

Add the timeout option from the requestIdleCallback specification. https://www.w3.org/TR/requestidlecallback/#the-requestidlecallback-method

Testing

I don't know where to put the test because expanding each file with client directives makes the tests very slow.

{/* Many many components */}
{Array.from({ length: 3000 }).map((_, i) => (<Counter id={`client-idle-${i}`} count={i} client:idle>
	<h1>Hello, client:idle! ${i}</h1>
</Counter>))}

{/* Component loads after a maximum wait of 500ms */}
<Counter id="client-idle-timeout" {...someProps} client:idle={{ timeout: 500 }}>
	<h1>Hello, client:idle={'{{timeout: 500}}'}!</h1>
</Counter>

Docs

client:idle={{timeout}}

Optionally, a value for timeout can be passed to the underlying requestIdleCallback. It hydrates the component if the client is not idle, but specifies the maximum time to wait.

<ShowHideButton client:idle={{ timeout: 500 }} />

withastro/docs#9130

Copy link

changeset-bot bot commented Aug 16, 2024

🦋 Changeset detected

Latest commit: 062cd46

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Aug 16, 2024
@ph1p ph1p force-pushed the feature/client-idle-options branch 2 times, most recently from 59e7eb0 to 94f8475 Compare August 16, 2024 14:14
@matthewp
Copy link
Contributor

Thanks! Can you add a test? One in the packages/astro/e2e folder would be preferred. I know you can't easily test that it triggers at the timeout, but even adding a test where the option is used will verify that the feature doesn't somehow cause hydration not to occur at all.

@ph1p
Copy link
Contributor Author

ph1p commented Aug 20, 2024

Sure! :)

@ph1p ph1p force-pushed the feature/client-idle-options branch from 8f122c4 to 13d00c1 Compare August 21, 2024 11:34
@ph1p
Copy link
Contributor Author

ph1p commented Aug 21, 2024

Done. I don't know why the PR checks are failing

@bluwy
Copy link
Member

bluwy commented Aug 21, 2024

I think you can ignore them. Those two don't work well with forks

@matthewp matthewp added this to the 4.15 milestone Aug 21, 2024
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thanks for this really helpful new feature addition, @ph1p ! I'm sure people will really appreciate this.

I've left some comments below for the changeset for your consideration, and I'll be reviewing the accompanying docs PR shortly!

.changeset/clever-emus-roll.md Outdated Show resolved Hide resolved
.changeset/clever-emus-roll.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Aug 22, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Approving for docs! Thank you @ph1p !

@ematipico ematipico merged commit cce0894 into withastro:main Aug 28, 2024
14 of 15 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants