fix: split env parsing into app/env.ts#151
Conversation
Agent-Logs-Url: https://github.com/mcansh/remix-wordle/sessions/841a4215-24bd-4908-9c77-51c5cfeab87e Co-authored-by: mcansh <11698668+mcansh@users.noreply.github.com>
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds client-side cheat activation via keyboard sequence detection (c-h-e-a-t within a 2-second window), refactors keyboard UI to semantic buttons with accessibility improvements, and reorganizes environment variable validation from Changes
Sequence DiagramsequenceDiagram
participant User
participant FormComponent as Form Component
participant SessionStorage
participant Handle
User->>FormComponent: Type keystroke
FormComponent->>FormComponent: Buffer keystroke (2s window)
alt Sequence matches "cheat"
FormComponent->>FormComponent: Enable cheatEnabled state
FormComponent->>SessionStorage: Set CHEAT_SESSION_KEY to "true"
alt handle provided & cheat was previously false
FormComponent->>Handle: Call handle.update()
end
else Sequence timeout or mismatch
FormComponent->>FormComponent: Reset buffer, no action
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
🚅 Deployed to the remix-wordle-pr-151 environment in remix-wordle
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/form.test.tsx`:
- Line 3: Replace the direct vitest imports with the repo-approved test wrapper
by changing the import source for the test utilities (afterEach, beforeEach,
describe, it, expect, vi) to "vite-plus/test" so all Vitest APIs are imported
via the centralized wrapper; update the import statement that currently imports
from "vitest" to import the same symbols from "vite-plus/test".
In `@app/components/form.tsx`:
- Around line 104-111: cheatEnabled is local and setting it won't re-render the
component unless handle.update() is always invoked; make the handle prop
required on GuessForm, update Page to pass its handle (rename _handle to handle)
into <GuessForm>, and remove the conditional so that handle.update() is called
unconditionally when enabling cheat (still write CHEAT_SESSION_KEY to
sessionStorage); also update GuessForm's prop types/signature to require handle
so the hidden input name="cheat" is rendered after activation.
In `@app/components/keyboard.test.tsx`:
- Line 3: Replace the direct import from "vitest" in keyboard.test.tsx with the
project test wrapper: update the import statement that currently references the
symbols describe, expect, it from "vitest" to instead import those same symbols
from "vite-plus/test" so the file uses the project's test utilities (locate the
import line at the top of the file and swap the module source).
In `@app/components/keyboard.tsx`:
- Around line 18-26: The keyboard buttons in app/components/keyboard.tsx render
as focusable <button>s but have no action; either make them
non-interactive/presentational or wire them to the same input handler: update
the element rendering inside the keyboard map (the element using letter.letter
and data-state) to either render a non-focusable element (e.g., span/div with
role="presentation" and no tabIndex) if you want passive display, or attach the
existing letter-entry handler (e.g., call insertLetter or onKeyPress equivalent)
to onClick and onKeyDown on the button, ensure it calls the same function used
for physical keyboard input, and keep accessible labels (aria-label) and
appropriate keyboard handling (Enter/Space) so the component remains accessible
and functional.
In `@app/controllers/home/controller.tsx`:
- Around line 68-70: Current logic only sets CHEAT_SESSION_KEY via
session.set(CHEAT_SESSION_KEY, true) when data.value.cheat is truthy, so the
server session never clears the flag when the client removes it; add server-side
handling to clear the flag as well: in the same request handler (the function
containing session.set and references to data.value.cheat) detect when
data.value.cheat is falsey and remove the key (e.g., session.delete or
session.unset/CHEAT_SESSION_KEY removal API) or explicitly set it to false, then
persist the session (commit/save) so the server-side session mirrors the client;
alternatively, add a dedicated server endpoint (e.g., /clear-cheat) that deletes
CHEAT_SESSION_KEY from the session and commits the session so the revealed
answer is no longer stored server-side.
In `@app/utils/db.ts`:
- Line 7: Replace the direct process.env usage with the validated value from
your env module: import the validated env export from app/env.ts and pass
env.DATABASE_URL into the PrismaPg constructor (replace the current
connectionString: process.env.DATABASE_URL). Update the file where adapter is
created (the PrismaPg instantiation) to reference env.DATABASE_URL so Prisma
uses the typed/validated config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8813aba2-c928-46e0-8dda-5288a8dad446
📒 Files selected for processing (10)
app/components/form.test.tsxapp/components/form.tsxapp/components/keyboard.test.tsxapp/components/keyboard.tsxapp/constants.tsapp/controllers/home/controller.tsxapp/env.tsapp/utils/db.tsapp/utils/queue.tsapp/utils/session.ts
| import { render, screen } from "@mcansh/remix-testing-library" | ||
| import { describe, it, expect, vi } from "vitest" | ||
| import { fireEvent, render, screen } from "@mcansh/remix-testing-library" | ||
| import { afterEach, beforeEach, describe, it, expect, vi } from "vitest" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'from\s+["'\''](?:vite|vitest)["'\'']' -g '*.{js,jsx,ts,tsx}'Repository: mcansh/remix-wordle
Length of output: 467
Import test utilities from vite-plus/test instead of vitest.
Line 3 imports directly from vitest, which violates the repository convention. Per coding guidelines, all **/*.{js,jsx,ts,tsx} files must import Vitest APIs from vite-plus/test.
Proposed fix
-import { afterEach, beforeEach, describe, it, expect, vi } from "vitest"
+import { afterEach, beforeEach, describe, it, expect, vi } from "vite-plus/test"📝 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 { afterEach, beforeEach, describe, it, expect, vi } from "vitest" | |
| import { afterEach, beforeEach, describe, it, expect, vi } from "vite-plus/test" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/form.test.tsx` at line 3, Replace the direct vitest imports
with the repo-approved test wrapper by changing the import source for the test
utilities (afterEach, beforeEach, describe, it, expect, vi) to "vite-plus/test"
so all Vitest APIs are imported via the centralized wrapper; update the import
statement that currently imports from "vitest" to import the same symbols from
"vite-plus/test".
| if (!cheatEnabled) { | ||
| cheatEnabled = true | ||
| if (typeof window !== "undefined") { | ||
| window.sessionStorage.setItem(CHEAT_SESSION_KEY, "true") | ||
| } | ||
| if (handle) { | ||
| await handle.update() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '<GuessForm\b|GuessForm\s*\(' -g '*.{js,jsx,ts,tsx}'Repository: mcansh/remix-wordle
Length of output: 5673
🏁 Script executed:
cat -n app/controllers/home/page.tsx | head -60Repository: mcansh/remix-wordle
Length of output: 2709
🏁 Script executed:
sed -n '100,130p' app/components/form.tsxRepository: mcansh/remix-wordle
Length of output: 702
🏁 Script executed:
rg -n "type Handle|interface Handle" -A5 --max-count=5Repository: mcansh/remix-wordle
Length of output: 45
🏁 Script executed:
sed -n '1,100p' app/components/form.tsxRepository: mcansh/remix-wordle
Length of output: 2659
🏁 Script executed:
rg "Handle" app/controllers/home/page.tsx -A2Repository: mcansh/remix-wordle
Length of output: 255
🏁 Script executed:
rg "handle\.update|Handle.*update" -A2 -B2Repository: mcansh/remix-wordle
Length of output: 1123
🏁 Script executed:
cat -n app/components/form.tsx | grep -A 5 "key=.*current-guess"Repository: mcansh/remix-wordle
Length of output: 257
Cheat activation will not trigger a render without passing the handle.
cheatEnabled is closure-local state; setting it on line 106 alone does not trigger a re-render. The hidden input on line 117 depends on cheatEnabled being true, but the component will not re-render unless handle.update() is called. However, the production caller at app/controllers/home/page.tsx:48 renders <GuessForm> without passing the handle parameter, so handle.update() is never executed. The form will submit without name="cheat" even after the cheat code is typed.
The _handle parameter in Page (line 10 of home/page.tsx) is intentionally unused (underscore prefix convention), indicating this limitation was deliberate. To fix this, the handle must be passed from Page to GuessForm, the optional parameter must become required, and the conditional call to handle.update() must be unconditional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/form.tsx` around lines 104 - 111, cheatEnabled is local and
setting it won't re-render the component unless handle.update() is always
invoked; make the handle prop required on GuessForm, update Page to pass its
handle (rename _handle to handle) into <GuessForm>, and remove the conditional
so that handle.update() is called unconditionally when enabling cheat (still
write CHEAT_SESSION_KEY to sessionStorage); also update GuessForm's prop
types/signature to require handle so the hidden input name="cheat" is rendered
after activation.
| @@ -0,0 +1,31 @@ | |||
| import "@testing-library/jest-dom/vitest" | |||
| import { render, screen } from "@mcansh/remix-testing-library" | |||
| import { describe, expect, it } from "vitest" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the import statement
cat -n app/components/keyboard.test.tsx | head -10Repository: mcansh/remix-wordle
Length of output: 457
🏁 Script executed:
# Check vite configuration and package.json to confirm vite-plus usage
head -30 vite.config.ts package.json 2>/dev/null || echo "Files not found at root"Repository: mcansh/remix-wordle
Length of output: 1576
🏁 Script executed:
# Look for vite-plus configuration or any imports from vite-plus in the project
rg "from ['\"]vite-plus" --type ts --type tsx -A 2 | head -20Repository: mcansh/remix-wordle
Length of output: 90
🏁 Script executed:
# Find other test files and check their imports
fd "\.test\.(ts|tsx|js|jsx)$" --exec head -10 {} \; | head -50Repository: mcansh/remix-wordle
Length of output: 1713
🏁 Script executed:
# Search for vite-plus/test imports specifically
rg "from.*vite-plus/test" --type ts --type jsx -A 1Repository: mcansh/remix-wordle
Length of output: 90
Use the project test wrapper instead of importing from vitest directly.
As per coding guidelines, **/*.{js,jsx,ts,tsx} must import test utilities from vite-plus/test instead of directly from vitest.
🧪 Proposed import fix
-import { describe, expect, it } from "vitest"
+import { describe, expect, it } from "vite-plus/test"📝 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 { describe, expect, it } from "vitest" | |
| import { describe, expect, it } from "vite-plus/test" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/keyboard.test.tsx` at line 3, Replace the direct import from
"vitest" in keyboard.test.tsx with the project test wrapper: update the import
statement that currently references the symbols describe, expect, it from
"vitest" to instead import those same symbols from "vite-plus/test" so the file
uses the project's test utilities (locate the import line at the top of the file
and swap the module source).
| <button | ||
| type="button" | ||
| data-state={letter.state} | ||
| class={`flex size-10 items-center justify-center rounded text-center text-white uppercase data-[state=Blank]:bg-gray-400 data-[state=Match]:bg-green-500 data-[state=Miss]:bg-red-500 data-[state=Present]:bg-yellow-500`} | ||
| key={`keyboard-letter-${letter.letter}`} | ||
| aria-label={`keyboard letter ${letter.letter}`} | ||
| > | ||
| {letter.letter} | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
Avoid rendering no-op buttons.
These controls are now focusable buttons, but they do not perform an action. Either wire them to the same letter-entry behavior as the physical keyboard or keep the keyboard cells non-interactive/presentational.
♿ Possible direction if these should be clickable
- <button
+ <button
type="button"
+ onClick={() => onLetter(letter.letter)}
data-state={letter.state}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/keyboard.tsx` around lines 18 - 26, The keyboard buttons in
app/components/keyboard.tsx render as focusable <button>s but have no action;
either make them non-interactive/presentational or wire them to the same input
handler: update the element rendering inside the keyboard map (the element using
letter.letter and data-state) to either render a non-focusable element (e.g.,
span/div with role="presentation" and no tabIndex) if you want passive display,
or attach the existing letter-entry handler (e.g., call insertLetter or
onKeyPress equivalent) to onClick and onKeyDown on the button, ensure it calls
the same function used for physical keyboard input, and keep accessible labels
(aria-label) and appropriate keyboard handling (Enter/Space) so the component
remains accessible and functional.
| if (data.value.cheat) { | ||
| session.set(CHEAT_SESSION_KEY, true) | ||
| } |
There was a problem hiding this comment.
Add a server-side path to clear the cheat flag.
Once Line 69 sets CHEAT_SESSION_KEY, Line 88 reveals the word for the rest of the server session. If the client can clear/expire the cheat state, the server session should mirror that; otherwise the answer stays revealed even after the client-side flag is gone.
🎯 Possible fix if the form always submits the current cheat state
- if (data.value.cheat) {
- session.set(CHEAT_SESSION_KEY, true)
- }
+ if (data.value.cheat) {
+ session.set(CHEAT_SESSION_KEY, true)
+ } else {
+ session.unset(CHEAT_SESSION_KEY)
+ }Also applies to: 88-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/controllers/home/controller.tsx` around lines 68 - 70, Current logic only
sets CHEAT_SESSION_KEY via session.set(CHEAT_SESSION_KEY, true) when
data.value.cheat is truthy, so the server session never clears the flag when the
client removes it; add server-side handling to clear the flag as well: in the
same request handler (the function containing session.set and references to
data.value.cheat) detect when data.value.cheat is falsey and remove the key
(e.g., session.delete or session.unset/CHEAT_SESSION_KEY removal API) or
explicitly set it to false, then persist the session (commit/save) so the
server-side session mirrors the client; alternatively, add a dedicated server
endpoint (e.g., /clear-cheat) that deletes CHEAT_SESSION_KEY from the session
and commits the session so the revealed answer is no longer stored server-side.
There was a problem hiding this comment.
Pull request overview
This PR refactors environment-variable parsing into a dedicated app/env.ts module and updates server utilities to consume it, while also introducing a new “cheat” persistence mechanism (session + sessionStorage) and adjusting UI/test behavior around cheating and the on-screen keyboard.
Changes:
- Move env schema/validation out of
app/constants.tsinto newapp/env.ts, updating imports across server utilities. - Change cheat behavior from a query-param-based reveal to a session-based flag, and add client-side “type cheat quickly” detection with persistence in
sessionStorage. - Update the keyboard UI to render letters as buttons and add/adjust component tests accordingly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/utils/session.ts | Updates env import to use the new app/env.ts. |
| app/utils/queue.ts | Updates env import to use the new app/env.ts. |
| app/utils/db.ts | Updates env import to use the new app/env.ts (but still uses process.env.DATABASE_URL for Prisma adapter). |
| app/env.ts | New centralized env schema and parsing. |
| app/controllers/home/controller.tsx | Switches cheat reveal from query param to session flag; adjusts redirect and reveal logic. |
| app/constants.ts | Removes env parsing and adds CHEAT_SESSION_KEY constant (leaves REVEAL_WORD behind). |
| app/components/keyboard.tsx | Renders keyboard letters as <button> with aria-label. |
| app/components/keyboard.test.tsx | Adds coverage verifying keyboard letters render as buttons. |
| app/components/form.tsx | Adds cheat-code key sequence detection + sessionStorage persistence and hidden input toggle. |
| app/components/form.test.tsx | Adds tests for cheat sequence timing and sessionStorage behavior. |
Comments suppressed due to low confidence (1)
app/utils/db.ts:8
- Now that env parsing has been centralized, this module still uses
process.env.DATABASE_URLwhen constructing the Prisma adapter. Usingenv.DATABASE_URLhere keeps validation/typing consistent and avoids bypassing the schema (and any future transforms/defaults).
import { env } from "#app/env.ts"
import { PrismaClient } from "#app/generated/prisma/client.ts"
const adapter = new PrismaPg({ connectionString: process.env.DATABASE_URL })
export const db = new PrismaClient({ adapter })
| let cheatInput = await screen.findByDisplayValue("true") | ||
| expect(cheatInput).toHaveAttribute("name", "cheat") | ||
| expect(window.sessionStorage.getItem("wordle-cheat-enabled")).toBe("true") | ||
| }) |
There was a problem hiding this comment.
This test hard-codes the sessionStorage key string ("wordle-cheat-enabled") instead of importing/using CHEAT_SESSION_KEY. Using the shared constant avoids test drift if the key changes.
| if (data.value.cheat) { | ||
| session.set(CHEAT_SESSION_KEY, true) | ||
| } | ||
|
|
||
| return redirect(routes.home.index.href()) |
There was a problem hiding this comment.
The PR title indicates this is only an env parsing refactor, but this diff also changes cheat behavior (query param -> session/sessionStorage) and UI semantics (keyboard keys become buttons) with new tests. Please update the PR title/description to reflect the broader scope, or split these functional changes into a separate PR to keep review/rollback risk contained.
| export const WORD_LENGTH = 5 | ||
| export const LETTER_INPUTS = [...Array(WORD_LENGTH).keys()] | ||
| export const TOTAL_GUESSES = 6 | ||
| export const REVEAL_WORD = "cheat" |
There was a problem hiding this comment.
REVEAL_WORD appears to be unused after switching cheat handling away from the query param (no other references in the app). Keeping an unused constant (and another separate CHEAT_CODE string elsewhere) makes the cheat behavior harder to follow—consider removing REVEAL_WORD or reusing it as the single source of truth for the cheat token.
| export const REVEAL_WORD = "cheat" |
| <button | ||
| type="button" | ||
| data-state={letter.state} | ||
| class={`flex size-10 items-center justify-center rounded text-center text-white uppercase data-[state=Blank]:bg-gray-400 data-[state=Match]:bg-green-500 data-[state=Miss]:bg-red-500 data-[state=Present]:bg-yellow-500`} | ||
| key={`keyboard-letter-${letter.letter}`} | ||
| aria-label={`keyboard letter ${letter.letter}`} | ||
| > | ||
| {letter.letter} | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
The keyboard letters are now rendered as <button> elements, but they have no click/press handler, so users can tab to “buttons” that do nothing. If the keyboard is meant to be non-interactive status, consider switching back to a non-control element (or mark the buttons disabled); if it is meant to be interactive, wire up a press/click behavior.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
267af85
into
copilot/add-keyboard-buttons-and-cheat-functionality
* Split env parsing into app/env.ts Agent-Logs-Url: https://github.com/mcansh/remix-wordle/sessions/841a4215-24bd-4908-9c77-51c5cfeab87e Co-authored-by: mcansh <11698668+mcansh@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> (cherry picked from commit 267af85)
Summary by CodeRabbit
New Features
Improvements