npm updates#227
Conversation
↑ @eslint/js 9.28.0 → 9.29.0 ↑ @sveltejs/kit 2.21.4 → 2.21.5 ↑ eslint 9.28.0 → 9.29.0 ↑ svelte 5.34.1 → 5.34.3
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change adds Playwright end-to-end testing setup including new npm test scripts, a Playwright configuration file, and a GitHub Actions workflow to run tests on pushes and pull requests. It also updates several devDependencies, modifies the sitemap.xml, updates Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
14-14: Document the newupscript in README.
Please update the project’s documentation (e.g. README or CONTRIBUTING) to describe how and when to usenpm run up(which runsbun update).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
package.json(2 hunks)
🔇 Additional comments (4)
package.json (4)
19-19: Bump @eslint/js to v9.29.0.
Patch update looks good.
22-22: Bump @sveltejs/kit to v2.21.5.
Minor release with no breaking changes noted upstream.
29-29: Bump eslint to v9.29.0.
Patch update is safe.
38-38: Bump svelte to v5.34.3.
Patch release—please ensure the full test suite and a quick manual smoke-test pass with this version.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
playwright.config.ts (1)
4-7: AddreuseExistingServerand explicit startup timeout for stabilityPlaywright may leave orphaned processes or hang if the server is already running or takes longer than the default 30 s to boot (common with a full
bun run build && bun run preview).
Consider extending the config:webServer: { command: 'bun run build && bun run preview', port: 4173, + reuseExistingServer: !process.env.CI, + timeout: 120 * 1000 // 2 min gives the build time to finish },This keeps local runs snappy while preventing CI flakes.
e2e/around-the-world.test.ts (2)
3-36: Collapse repetitive navigation into a data-driven loopThe test repeats the same three lines for every route, which is verbose and hard to maintain. Use an array and
for…of:-test('can load all pages', async ({ page }) => { - await page.goto('/'); - await expect(page.locator('h1')).toBeVisible(); - … - await page.goto('/policies'); - await expect(page.locator('h1')).toBeVisible(); -}); +const routes = [ + '/', + '/blog', + '/blog/screenshot-maker', + '/blog/pagespeed-testing', + '/blog/github-repositories-viewer-app', + '/blog/git-branch-name-raycast-extension', + '/blog/i-made-an-extension', + '/blog/getting-started-as-a-web-developer-in-2022', + '/blog/set-up-some-aliases', + '/uses', + '/policies' +]; + +test('all pages render an <h1>', async ({ page }) => { + for (const path of routes) { + await page.goto(path); + await expect(page.locator('h1'), path).toBeVisible(); + } +});Benefits: easier additions/removals, single failure pinpoints failing route.
3-36: Assert heading text, not just visibility, for stronger guaranteeVisibility alone doesn’t catch wrong template/render errors where an empty
<h1>is injected.
Consider asserting expected text for each route, e.g.:await expect(page.locator('h1')).toHaveText(/Around the World/i);(or keep a map of route → expected text).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/playwright.yml(1 hunks).gitignore(2 hunks)e2e/around-the-world.test.ts(1 hunks)package.json(2 hunks)playwright.config.ts(1 hunks)static/sitemap.xml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- .github/workflows/playwright.yml
- static/sitemap.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
e2e/around-the-world.test.ts (1)
3-36: Tie test timeout to navigation instead of global defaultSome blog pages may load heavy assets and occasionally exceed the Playwright default 30 s test timeout. Explicitly set navigation timeout or rely on
expect().toBeVisible({ timeout: … })to avoid flaky failures.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/routes/+layout.svelte (1)
38-48:⚠️ Potential issueAnalytics enabled by default – potential privacy & compliance issue
Current logic:
if (PUBLIC_POSTHOG_ENABLED !== 'false' && browser) { … }Problems
- Missing or mistyped env var ⇒ analytics ON (unexpected in staging / CI).
- Empty
PUBLIC_POSTHOG_API_KEYstill callsposthog.init(''), wasting bytes and logging console errors.Recommended change:
-if (PUBLIC_POSTHOG_ENABLED !== 'false' && browser) { +if (browser && PUBLIC_POSTHOG_ENABLED === 'true' && PUBLIC_POSTHOG_API_KEY) {Optionally defer the heavy dependency:
if (browser && PUBLIC_POSTHOG_ENABLED === 'true' && PUBLIC_POSTHOG_API_KEY) { const { default: posthog } = await import('posthog-js'); posthog.init(PUBLIC_POSTHOG_API_KEY, { /* … */ }); }This guarantees explicit opt-in, avoids accidental PII capture, and trims ~20 kB from the default bundle.
🧹 Nitpick comments (2)
.env.example (1)
2-2: Minor: add trailing newlineThe dotenv-linter warning is valid; append a blank line to keep linters quiet.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 2-2: [EndingBlankLine] No blank line at the end of the file
src/routes/+layout.svelte (1)
7-8: Import order / unused import check
PUBLIC_POSTHOG_API_KEYis only referenced inside the analytics block.
If you decide to lazy-load PostHog (see next comment) you can also move this import into that branch to keep the main bundle smaller.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.env.example(1 hunks).github/workflows/playwright.yml(1 hunks)playwright.config.ts(1 hunks)postcss.config.cjs(1 hunks)postcss.config.mjs(0 hunks)src/app.css(1 hunks)src/routes/+layout.svelte(2 hunks)
💤 Files with no reviewable changes (1)
- postcss.config.mjs
✅ Files skipped from review due to trivial changes (2)
- postcss.config.cjs
- src/app.css
🚧 Files skipped from review as they are similar to previous changes (2)
- playwright.config.ts
- .github/workflows/playwright.yml
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 2-2: [EndingBlankLine] No blank line at the end of the file
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
| PUBLIC_POSTHOG_API_KEY= | ||
| PUBLIC_POSTHOG_ENABLED=false No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Default-off toggle can back-fire if the variable is missing
In +layout.svelte analytics are enabled unless PUBLIC_POSTHOG_ENABLED === 'false'.
If this line is absent from a real .env file (easy to forget when onboarding or in CI) PostHog will be turned on unintentionally. Safer pattern:
-PUBLIC_POSTHOG_ENABLED=false
+# Explicitly opt-in. Any value other than “true” disables analytics.
+PUBLIC_POSTHOG_ENABLED=trueand flip the check in the Svelte file to === 'true'.
This prevents accidental PII leakage and makes the intent crystal-clear.
📝 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.
| PUBLIC_POSTHOG_API_KEY= | |
| PUBLIC_POSTHOG_ENABLED=false | |
| PUBLIC_POSTHOG_API_KEY= | |
| # Explicitly opt-in. Any value other than “true” disables analytics. | |
| PUBLIC_POSTHOG_ENABLED=true |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 2-2: [EndingBlankLine] No blank line at the end of the file
🤖 Prompt for AI Agents
In the .env.example file at lines 1 to 2, change the default value of
PUBLIC_POSTHOG_ENABLED from false to true or remove it so that the variable must
be explicitly set to 'true' to enable analytics. Then, update the check in
+layout.svelte to enable analytics only when PUBLIC_POSTHOG_ENABLED === 'true'.
This ensures analytics are off by default and only enabled intentionally.
1 participant
↑ @eslint/js 9.28.0 → 9.29.0
↑ @sveltejs/kit 2.21.4 → 2.21.5
↑ eslint 9.28.0 → 9.29.0
↑ svelte 5.34.1 → 5.34.3
Summary by CodeRabbit