Add /dev/icons to preview icons during development#929
Add /dev/icons to preview icons during development#929chrisvire merged 1 commit intosillsdev:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a development-only SvelteKit page at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/routes/dev/icons/`+page.svelte:
- Around line 94-96: The Tailwind class on the card div uses an invalid
arbitrary value "min-w-[250]"—update the class in the component containing the
div (the element with classes "flex flex-col items-center p-4 border rounded-lg
hover:bg-gray-50 transition-colors min-w-[250]") to include a unit, e.g. change
"min-w-[250]" to "min-w-[250px]" (or another appropriate unit like rem or %) so
the min-width is a valid CSS length.
- Around line 3-10: The current top-level call to goto when dev is false runs
during SSR and crashes because goto is browser-only; to fix, guard the
client-only redirect by importing browser from '$app/environment' and only call
goto inside a browser check (or move the redirect logic into a server route
using a server-side redirect in +page.server.ts). Specifically, update the file
to check browser && !dev before calling goto (or alternatively implement a
server-side redirect in +page.server.ts using redirect(...)) and ensure any
existing onMount usage contains the same browser guard so navigation only runs
in the client.
| import { dev } from '$app/environment'; | ||
| import { goto } from '$app/navigation'; | ||
| import { onMount } from 'svelte'; | ||
|
|
||
| // Redirect to home if not in development mode | ||
| if (!dev) { | ||
| goto('/'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get the full file content to see the min-w-[250] issue and full context
find . -name "+page.svelte" -path "*/dev/icons/*" -type fRepository: sillsdev/appbuilder-pwa
Length of output: 102
🏁 Script executed:
# Search for the specific file mentioned
git ls-files | grep -E "dev/icons/\+page\.svelte"Repository: sillsdev/appbuilder-pwa
Length of output: 100
🏁 Script executed:
# Check if the file exists and its content
fd "+page.svelte" | grep "dev/icons"Repository: sillsdev/appbuilder-pwa
Length of output: 358
🌐 Web query:
SvelteKit goto function SSR browser-only behavior and server-side redirect patterns
💡 Result:
Why goto() is “browser-only” (and what happens in SSR)
goto(from$app/navigation) is client-side navigation. During SSR there is no browser history to manipulate, so calling it while rendering on the server results in errors like “Cannot call goto(...) on the server”. In SvelteKit, component<script>blocks execute during SSR, so top-levelgoto(...)(or logic that runs during render) can accidentally run on the server. Usegotoonly in code that you know runs in the browser (e.g., insideonMount, event handlers), or guard withbrowserfrom$app/environment. The docs explicitly positiongotoas the browser/programmatic navigation API, while redirects during request handling useredirect. [1]
Server-side redirect (SSR-safe) patterns (recommended)
Use redirect from @sveltejs/kit anywhere SvelteKit is handling a request (SSR, endpoints, actions, hooks):
1) Protect a route (auth guard) in +layout.server.ts / +page.server.ts
// src/routes/(app)/+layout.server.ts
import { redirect } from '@sveltejs/kit';
export const load = ({ locals, url }) => {
if (!locals.user) {
const redirectTo = url.pathname + url.search;
redirect(307, `/login?redirectTo=${encodeURIComponent(redirectTo)}`);
}
};This is the canonical SSR guard: if the user isn’t allowed, the server responds with a real HTTP redirect. [1]
2) Redirect after a form POST in +page.server.ts actions
// src/routes/login/+page.server.ts
import { redirect, fail } from '@sveltejs/kit';
export const actions = {
default: async ({ request, cookies }) => {
// ...validate...
// ...set cookies/session...
redirect(303, '/dashboard'); // 303 is common after POST
}
};Redirects in actions work the same way as in load. [2]
3) Redirect from endpoints (+server.ts) or hooks (hooks.server.ts)
You can also redirect(...) during request handling in endpoints/hooks (same helper, same behavior). [3]
Status code guidance (practical defaults)
SvelteKit’s redirect(status, location) docs call out common choices:
- 303 See Other: typical after successful form submissions (POST → GET)
- 307 Temporary Redirect: preserves method (and is a common “login gate” default)
- 308 Permanent Redirect: preserves method + permanent [3]
“Do I need throw redirect(...)?”
- SvelteKit v2: calling
redirect(...)is sufficient; it throws internally. [1][4]
(If you’re reading older v1-era examples, they often showthrow redirect(...).)
Avoid this common pitfall
- Don’t wrap
redirect()in a broadtry/catch, because it throws as control flow; catching it will prevent SvelteKit from performing the redirect (unless you rethrow usingisRedirect). The docs explicitly warn about this. [1][4]
Sources:
[1] SvelteKit docs: load → Redirects + note about goto for browser navigation https://svelte.dev/docs/kit/load
[2] SvelteKit docs: Form actions → Redirects https://svelte.dev/docs/kit/form-actions
[3] SvelteKit API docs: redirect helper + common status codes https://svelte.jp/docs/kit/%40sveltejs-kit
[4] SvelteKit docs: Migrating to v2 → “redirect and error are no longer thrown by you” https://svelte.dev/docs/kit/migrating-to-sveltekit-2
🏁 Script executed:
# Get the full content of the file
cat -n src/routes/dev/icons/+page.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 6445
🏁 Script executed:
# Check if there's a +page.server.ts file for this route
ls -la src/routes/dev/icons/Repository: sillsdev/appbuilder-pwa
Length of output: 239
🏁 Script executed:
# Search for similar dev-only route patterns in the codebase
rg "if.*\(!?dev\)" --type svelte -A 2Repository: sillsdev/appbuilder-pwa
Length of output: 97
🏁 Script executed:
# Check if there are other routes using +page.server.ts with redirect pattern
fd "+page.server.ts" | head -5 | xargs cat -nRepository: sillsdev/appbuilder-pwa
Length of output: 361
🏁 Script executed:
# Verify Tailwind min-w syntax requirements by checking if similar classes exist in codebase
rg 'min-w-\[' src/ --type svelte | head -10Repository: sillsdev/appbuilder-pwa
Length of output: 97
🏁 Script executed:
# Check if there are other routes handling dev-only access to understand codebase patterns
rg 'if.*dev\)' src/routes --type svelte -B 1 -A 2Repository: sillsdev/appbuilder-pwa
Length of output: 97
Avoid SSR crash from goto in non‑dev builds
goto is browser‑only and executes during SSR when dev === false, causing a server error. Guard with browser from $app/environment or move the redirect to +page.server.ts using redirect(...).
Suggested fix (client-side guard)
-import { dev } from '$app/environment';
+import { browser, dev } from '$app/environment';
import { goto } from '$app/navigation';
import { onMount } from 'svelte';
// Redirect to home if not in development mode
-if (!dev) {
+if (!dev && browser) {
goto('/');
}📝 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.
| import { dev } from '$app/environment'; | |
| import { goto } from '$app/navigation'; | |
| import { onMount } from 'svelte'; | |
| // Redirect to home if not in development mode | |
| if (!dev) { | |
| goto('/'); | |
| } | |
| import { browser, dev } from '$app/environment'; | |
| import { goto } from '$app/navigation'; | |
| import { onMount } from 'svelte'; | |
| // Redirect to home if not in development mode | |
| if (!dev && browser) { | |
| goto('/'); | |
| } |
🤖 Prompt for AI Agents
In `@src/routes/dev/icons/`+page.svelte around lines 3 - 10, The current top-level
call to goto when dev is false runs during SSR and crashes because goto is
browser-only; to fix, guard the client-only redirect by importing browser from
'$app/environment' and only call goto inside a browser check (or move the
redirect logic into a server route using a server-side redirect in
+page.server.ts). Specifically, update the file to check browser && !dev before
calling goto (or alternatively implement a server-side redirect in
+page.server.ts using redirect(...)) and ensure any existing onMount usage
contains the same browser guard so navigation only runs in the client.
| <div | ||
| class="flex flex-col items-center p-4 border rounded-lg hover:bg-gray-50 transition-colors min-w-[250]" | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's verify the file exists and check the code at the specified lines
fd "+page.svelte" src/routes/dev/icons/Repository: sillsdev/appbuilder-pwa
Length of output: 358
🌐 Web query:
Tailwind CSS arbitrary value syntax min-w utility units requirements
💡 Result:
- Arbitrary value syntax for
min-width: usemin-w-[<value>], e.g.min-w-[220px]ormin-w-[50%]. Tailwind will emitmin-width: <value>as-is, so the bracket contents must be a valid CSSmin-widthvalue. [1] - CSS variable shorthand (Tailwind v4+):
min-w-(--my-min-width)is shorthand formin-w-[var(--my-min-width)]. [1] - Units requirement: because
min-widthaccepts<length>or<percentage>(and some keywords/functions), a plain number likemin-w-[220]is invalid CSS—usemin-w-[220px],min-w-[13.75rem],min-w-[10%], etc.0can be unitless (min-w-[0]). [1][3] - Other valid arbitrary values: keywords like
min-w-[max-content],min-w-[min-content],min-w-[fit-content], and functions likemin-w-[fit-content(20em)],min-w-[calc(100%-2rem)], etc., as long as they’re valid formin-width. [2][3]
Sources: Tailwind min-width docs [1], Tailwind arbitrary values docs [2], MDN min-width reference [3].
🏁 Script executed:
# Use --fixed-strings to avoid regex parsing issues
fd --fixed-strings "+page.svelte" src/Repository: sillsdev/appbuilder-pwa
Length of output: 816
🏁 Script executed:
# Check the file size first
wc -l src/routes/dev/icons/+page.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 104
🏁 Script executed:
# Read lines around 94-96 to verify the code snippet
sed -n '90,100p' src/routes/dev/icons/+page.svelteRepository: sillsdev/appbuilder-pwa
Length of output: 633
Fix invalid Tailwind arbitrary value
min-w-[250] is missing a unit—the CSS min-width property requires a valid <length> or <percentage> value. Change to min-w-[250px] (or another appropriate unit like rem, %, etc.).
🩹 Proposed fix
- class="flex flex-col items-center p-4 border rounded-lg hover:bg-gray-50 transition-colors min-w-[250]"
+ class="flex flex-col items-center p-4 border rounded-lg hover:bg-gray-50 transition-colors min-w-[250px]"📝 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.
| <div | |
| class="flex flex-col items-center p-4 border rounded-lg hover:bg-gray-50 transition-colors min-w-[250]" | |
| > | |
| <div | |
| class="flex flex-col items-center p-4 border rounded-lg hover:bg-gray-50 transition-colors min-w-[250px]" | |
| > |
🤖 Prompt for AI Agents
In `@src/routes/dev/icons/`+page.svelte around lines 94 - 96, The Tailwind class
on the card div uses an invalid arbitrary value "min-w-[250]"—update the class
in the component containing the div (the element with classes "flex flex-col
items-center p-4 border rounded-lg hover:bg-gray-50 transition-colors
min-w-[250]") to include a unit, e.g. change "min-w-[250]" to "min-w-[250px]"
(or another appropriate unit like rem or %) so the min-width is a valid CSS
length.
chrisvire
force-pushed
the
feature/dev-icon-gallery
branch
from
eaec132 to
5b07dda
Compare
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.