-
Notifications
You must be signed in to change notification settings - Fork 301
Blog: Best practices for handling screenshots in your app #2714
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
Conversation
WalkthroughA new documentation page was added at Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Pull request overview
Adds a new blog post documenting best practices for efficiently capturing, storing, and refreshing webpage screenshots in an app.
Changes:
- Added a new Markdoc blog post: “Best practices for handling screenshots in your app”
- Included code examples for persistence (Storage), lookup (TablesDB), refresh strategies, and screenshot parameter tuning
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // No existing screenshot, create new one | ||
| return await createScreenshot(url); |
Copilot
AI
Jan 26, 2026
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 function returns createScreenshot(url), but createScreenshot isn’t defined anywhere in the post. Either define it or reuse the earlier screenshot creation logic so the snippet is complete.
| return await createScreenshot(url); | |
| const fileId = await captureAndStore(url); | |
| return storage.getFileView('screenshots', fileId); |
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/routes/blog/post/screenshots-best-practices/`+page.markdoc:
- Around line 74-79: The snippet uses client but never defines it; add a brief
initialization before using Client/Storage/TablesDB/Avatars (e.g., create and
configure a Client instance and assign it to the variable client) or add a
one-line note that client is assumed to be defined earlier; ensure the symbol
client is present and configured before the lines referencing new
Storage(client), new TablesDB(client), and new Avatars(client) so the snippet
can be copied and run independently.
- Around line 121-153: The example uses undefined helper functions
captureAndStore and createScreenshot in getScreenshot, making the snippet
non-runnable; add brief placeholder implementations or a clarifying note.
Specifically, either add short helper descriptions or minimal implementations
for captureAndStore(url) and createScreenshot(url) (and ensure they return a
fileId or call storage.getFileView as used), or prepend a comment to the
getScreenshot example stating these are stubs/placeholders that must be
implemented (referencing the function name getScreenshot so readers can locate
it). Ensure the helpers' return shapes match the rest of the snippet (fileId or
storage view) so the updateRow and storage.getFileView calls work.
🧹 Nitpick comments (1)
src/routes/blog/post/screenshots-best-practices/+page.markdoc (1)
255-264: Add a server-side safety note for auth headers.The
Authorization/Cookieheaders example should explicitly state this must be executed server-side (to avoid exposing secrets, and because browsers block setting theCookieheader). A short caution line would prevent misuse.💡 Suggested addition
+// Run this server-side; do not expose tokens or cookies in client-side code. const authenticatedScreenshot = avatars.getScreenshot({
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| } | ||
|
|
||
| // No existing screenshot, create new one | ||
| return await createScreenshot(url); |
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 pages with heavy JavaScript, 2-3 seconds is usually sufficient. For simple static pages, you can skip the delay entirely. |
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 improve that first part and include a mention of loading
Also, pages with heavy JavaScript is a little too informal
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.
Pull request overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/routes/blog/post/screenshots-best-practices/`+page.markdoc:
- Line 246: Add a short server-side security caution after the line "For
authenticated pages, use the `headers` parameter to pass authorization tokens or
session cookies:" clarifying that tokens should never be embedded in client-side
or browser-exposed code; recommend sending auth tokens from server-side code
(e.g., in a load function or API route) and passing only necessary session data
to the client to avoid exposing secrets. Reference the `headers` parameter and
the phrase "authenticated pages" so the note is placed immediately afterward.
🧹 Nitpick comments (1)
src/routes/blog/post/screenshots-best-practices/+page.markdoc (1)
272-276: Avoid duplicate Screenshots API links.The Screenshots API documentation is linked in the paragraph and again in “Further reading.” Consider removing the duplicate to reduce redundancy.
🧹 Minimal cleanup
- - [Screenshots API documentation](/docs/products/avatars/screenshots)
| }); | ||
| ``` | ||
|
|
||
| For authenticated pages, use the `headers` parameter to pass authorization tokens or session cookies: |
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.
Add a server-side security note for auth headers.
The sentence can be read as client-side guidance. Please add a short caution to keep tokens server-side to avoid exposing secrets in browser code.
🛡️ Suggested wording
-For authenticated pages, use the `headers` parameter to pass authorization tokens or session cookies:
+For authenticated pages, use the `headers` parameter to pass authorization tokens or session cookies. Do this from a trusted backend/service to avoid exposing secrets in client-side code:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| For authenticated pages, use the `headers` parameter to pass authorization tokens or session cookies: | |
| For authenticated pages, use the `headers` parameter to pass authorization tokens or session cookies. Do this from a trusted backend/service to avoid exposing secrets in client-side code: |
🤖 Prompt for AI Agents
In `@src/routes/blog/post/screenshots-best-practices/`+page.markdoc at line 246,
Add a short server-side security caution after the line "For authenticated
pages, use the `headers` parameter to pass authorization tokens or session
cookies:" clarifying that tokens should never be embedded in client-side or
browser-exposed code; recommend sending auth tokens from server-side code (e.g.,
in a load function or API route) and passing only necessary session data to the
client to avoid exposing secrets. Reference the `headers` parameter and the
phrase "authenticated pages" so the note is placed immediately afterward.
Co-authored-by: Aditya Oberai <adityaoberai1@gmail.com>
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.