-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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(audits): Add initial perf audits #10015
Conversation
🦋 Changeset detectedLatest commit: bfdf79b 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 |
⚖️ Bundle Size CheckLatest commit: bfdf79b
|
There was a problem hiding this 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.
const hydrator = this.hydrator(this); | ||
if (process.env.NODE_ENV === 'development') hydrationTimeStart = performance.now(); | ||
await this.hydrator(this)(this.Component, props, slots, { | ||
await hydrator(this.Component, props, slots, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this actually does something, but when testing locally, I was getting times that made a bit more sense and were generally more stable 🤷♀️
Co-authored-by: Bjorn Lu <[email protected]>
packages/astro/e2e/fixtures/dev-toolbar/src/pages/audits-perf.astro
Outdated
Show resolved
Hide resolved
packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts
Outdated
Show resolved
Hide resolved
packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts
Outdated
Show resolved
Hide resolved
packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts
Outdated
Show resolved
Hide resolved
title: 'Use videos instead of GIFs for large animations', | ||
message: | ||
'This GIF could be replaced with a video to reduce its file size and improve performance.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should tell the user what "large" means, maybe by explicitly tell the user what's the threshold of this audit
packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts
Outdated
Show resolved
Hide resolved
packages/astro/src/runtime/client/dev-toolbar/apps/audit/perf.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of a visible checkmark, docs is approving!
There was a problem hiding this 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.
Changes
This PR adds a beginning of perf audits and infrastructure for it, most notably, this includes a
server-render-time
andclient-render-time
attribute on islands, allowing us to provide audits when an island take a suspicious amount of time to render.Testing
Added some tests!
Docs
N/A