-
Notifications
You must be signed in to change notification settings - Fork 0
Persist todos in localStorage + minimal storage utils (STA-2) #7
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
Changes from all commits
f384661
9d7e070
f101f1c
ee570f6
2b256ca
b2b1c85
bbd89d0
1032007
f4c3484
25ea083
080a42b
09af38e
1bb8154
5279de1
5985235
01f7400
3d76fe0
af9cb46
ca43444
ca3ebb3
74ea542
e59dd38
37dd5bc
e2bde01
14addb0
b4a12cc
ec3dbc5
24a407c
57bd96c
f28d317
165c756
e6a7c59
60bd6cf
4b16a9a
d0927af
1daac84
b034ea2
a5386c5
fdeb1c8
e70bd83
ff9f73a
fd3755f
eff66cd
62a178d
6d7b289
b97e8cc
cc1db4f
406bfd2
25f4472
3a52810
eb25867
bdf695d
b12ecc7
98f8b92
501a061
c7b40a2
9a7b467
ca5d860
6cfffe6
e67690e
db102d2
09fa3ce
ab920c1
8b2d28b
073512b
926fad4
2cfaab9
721508f
da4c5f9
79e7e20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ on: | |||||||||||||||||||||
| - 'bun.lock' | ||||||||||||||||||||||
| - 'turbo.json' | ||||||||||||||||||||||
| - 'biome.json' | ||||||||||||||||||||||
| - '.github/workflows/**' | ||||||||||||||||||||||
| - '!**/*.md' | ||||||||||||||||||||||
| - '!**/*.txt' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -33,28 +34,16 @@ jobs: | |||||||||||||||||||||
| with: | ||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Setup Bun | ||||||||||||||||||||||
| uses: oven-sh/setup-bun@v2 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| bun-version: '1.2.19' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Cache dependencies | ||||||||||||||||||||||
| uses: actions/cache@v4 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| path: ~/.bun | ||||||||||||||||||||||
| key: ${{ runner.os }}-deps-${{ hashFiles('**/bun.lock') }} | ||||||||||||||||||||||
| restore-keys: | | ||||||||||||||||||||||
| ${{ runner.os }}-deps- | ||||||||||||||||||||||
| - name: Install dependencies | ||||||||||||||||||||||
| run: bun install --frozen-lockfile | ||||||||||||||||||||||
| - name: Setup Bun and install dependencies | ||||||||||||||||||||||
| uses: ./.github/composite/bun-install | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - name: Cache Turbo | ||||||||||||||||||||||
| uses: actions/cache@v4 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| path: .turbo | ||||||||||||||||||||||
| key: ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ github.sha }} | ||||||||||||||||||||||
| key: ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ hashFiles('**/bun.lockb') }}-${{ github.sha }} | ||||||||||||||||||||||
| restore-keys: | | ||||||||||||||||||||||
| ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ hashFiles('**/bun.lockb') }}- | ||||||||||||||||||||||
| ${{ runner.os }}-turbo-${{ github.ref_name }}- | ||||||||||||||||||||||
| ${{ runner.os }}-turbo- | ||||||||||||||||||||||
|
Comment on lines
+44
to
48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainConfirm bun.lockb presence across the monorepo for cache key effectiveness Turbo cache keys now hash Run to verify presence and consistency: 🏁 Script executed: #!/bin/bash
# List Bun lockfiles and their paths
fd -HI --regex 'bun\.lockb?$' | sort
# Show which workflows reference bun.lock vs bun.lockb
rg -n "bun\.lockb?" -A 2 -B 2 .github/workflowsLength of output: 1631 Use bun.lock for Turbo cache keys in workflows We only have bun.lock in the repo (no bun.lockb), so the current • .github/workflows/pr-quality-checks.yml (lines 44–48) Suggested diff for each: - key: ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ hashFiles('**/bun.lockb') }}-${{ github.sha }}
+ key: ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ hashFiles('**/bun.lock') }}-${{ github.sha }}
- restore-keys: |
- ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ hashFiles('**/bun.lockb') }}-
+ restore-keys: |
+ ${{ runner.os }}-turbo-${{ github.ref_name }}-${{ hashFiles('**/bun.lock') }}-
${{ runner.os }}-turbo-${{ github.ref_name }}-
${{ runner.os }}-turbo-📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
@@ -78,4 +67,3 @@ jobs: | |||||||||||||||||||||
| echo "✅ TypeScript compilation passed" >> $GITHUB_STEP_SUMMARY | ||||||||||||||||||||||
| echo "✅ Tests passed" >> $GITHUB_STEP_SUMMARY | ||||||||||||||||||||||
| echo "✅ All checks completed with Turbo caching" >> $GITHUB_STEP_SUMMARY | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,63 +1,187 @@ | ||
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { AddTodo } from '../add-todo'; | ||
| import { createMemoryRouter, RouterProvider } from 'react-router-dom'; | ||
| import type { ReactElement, ReactNode, ChangeEvent, FormEvent } from 'react'; | ||
|
|
||
| // Create a stateful mock for the input field | ||
| let testInputValue = ''; | ||
|
|
||
| // Mock lucide-react icons | ||
| vi.mock('lucide-react', () => ({ | ||
| Plus: () => null | ||
| })); | ||
|
|
||
| // Mock the @lambdacurry/forms components | ||
| interface TextFieldProps { | ||
| name: string; | ||
| placeholder: string; | ||
| className: string; | ||
| } | ||
|
|
||
| vi.mock('@lambdacurry/forms', () => ({ | ||
| TextField: ({ name, placeholder, className }: TextFieldProps) => ( | ||
| <input | ||
| name={name} | ||
| placeholder={placeholder} | ||
| className={className} | ||
| type="text" | ||
| value={testInputValue} | ||
| onChange={e => { | ||
| testInputValue = e.target.value; | ||
| }} | ||
| /> | ||
| ), | ||
| FormError: () => null | ||
| })); | ||
|
|
||
| interface ButtonProps { | ||
| children: ReactNode; | ||
| onClick: () => void; | ||
| type: 'button' | 'submit' | 'reset'; | ||
| } | ||
|
|
||
| vi.mock('@lambdacurry/forms/ui', () => ({ | ||
| Button: ({ children, onClick, type }: ButtonProps) => ( | ||
| <button type={type} onClick={onClick}> | ||
| {children} | ||
| </button> | ||
| ) | ||
| })); | ||
|
|
||
| // Mock the remix-hook-form module | ||
| interface RemixFormConfig { | ||
| submitHandlers?: { | ||
| onValid: (data: { text: string }) => void; | ||
| }; | ||
| [key: string]: unknown; | ||
| } | ||
|
|
||
| vi.mock('remix-hook-form', () => { | ||
| let latestConfig: RemixFormConfig | undefined; | ||
| return { | ||
| RemixFormProvider: ({ children }: { children: ReactNode }) => children, | ||
| useRemixForm: (config: RemixFormConfig) => { | ||
| latestConfig = config; | ||
| return { | ||
| ...config, | ||
| getValues: (_name: string) => testInputValue, | ||
| reset: vi.fn(() => { | ||
| testInputValue = ''; | ||
| // Force re-render by dispatching a custom event | ||
| const inputs = document.querySelectorAll('input[name="text"]'); | ||
| inputs.forEach(input => { | ||
| (input as HTMLInputElement).value = ''; | ||
| }); | ||
| }), | ||
| setValue: vi.fn((_name: string, value: string) => { | ||
| testInputValue = value; | ||
| }), | ||
| register: vi.fn((name: string) => ({ | ||
| name, | ||
| onChange: (e: ChangeEvent<HTMLInputElement>) => { | ||
| testInputValue = e.target.value; | ||
| }, | ||
| value: testInputValue | ||
| })), | ||
| handleSubmit: vi.fn((arg?: unknown) => { | ||
| // Support both usages: | ||
| // 1) onSubmit={methods.handleSubmit} → arg is the FormEvent | ||
| // 2) onSubmit={methods.handleSubmit(onValid)} → arg is the onValid callback | ||
| const isEvent = arg && typeof (arg as FormEvent).preventDefault === 'function'; | ||
| if (isEvent) { | ||
| const e = arg as FormEvent; | ||
| e.preventDefault(); | ||
| const onValid = latestConfig?.submitHandlers?.onValid; | ||
| if (onValid && testInputValue?.trim()) onValid({ text: testInputValue.trim() }); | ||
| return undefined; | ||
| } | ||
| const maybeOnValid = arg as ((data: { text: string }) => void) | undefined; | ||
| return (e: FormEvent) => { | ||
| e.preventDefault(); | ||
| const onValid = maybeOnValid || latestConfig?.submitHandlers?.onValid; | ||
| if (onValid && testInputValue?.trim()) onValid({ text: testInputValue.trim() }); | ||
| }; | ||
| }), | ||
| formState: { errors: {} }, | ||
| watch: vi.fn((_name: string) => testInputValue) | ||
| }; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| function renderWithRouter(ui: ReactElement) { | ||
| const router = createMemoryRouter([{ path: '/', element: ui }], { initialEntries: ['/'] }); | ||
| return render(<RouterProvider router={router} />); | ||
| } | ||
|
|
||
| // hoist regex literals to top-level to satisfy biome's useTopLevelRegex | ||
| const ADD_REGEX = /add/i; | ||
|
|
||
| describe('AddTodo', () => { | ||
| beforeEach(() => { | ||
| // Reset the test state before each test | ||
| testInputValue = ''; | ||
| }); | ||
|
|
||
| it('renders input and button', () => { | ||
| const mockOnAdd = vi.fn(); | ||
| render(<AddTodo onAdd={mockOnAdd} />); | ||
| renderWithRouter(<AddTodo onAdd={mockOnAdd} />); | ||
|
|
||
| expect(screen.getByPlaceholderText('Add a new todo...')).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', { name: /add/i })).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', { name: ADD_REGEX })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('calls onAdd when form is submitted with text', () => { | ||
| const mockOnAdd = vi.fn(); | ||
| render(<AddTodo onAdd={mockOnAdd} />); | ||
| renderWithRouter(<AddTodo onAdd={mockOnAdd} />); | ||
|
|
||
| const input = screen.getByPlaceholderText('Add a new todo...'); | ||
| const button = screen.getByRole('button', { name: /add/i }); | ||
|
|
||
| const button = screen.getByRole('button', { name: ADD_REGEX }); | ||
| const form = button.closest('form') as HTMLFormElement; | ||
|
|
||
| fireEvent.change(input, { target: { value: 'New todo' } }); | ||
| fireEvent.click(button); | ||
| fireEvent.submit(form); | ||
|
|
||
| expect(mockOnAdd).toHaveBeenCalledWith('New todo'); | ||
| }); | ||
|
|
||
| it('clears input after adding todo', () => { | ||
| const mockOnAdd = vi.fn(); | ||
| render(<AddTodo onAdd={mockOnAdd} />); | ||
| renderWithRouter(<AddTodo onAdd={mockOnAdd} />); | ||
|
|
||
| const input = screen.getByPlaceholderText('Add a new todo...') as HTMLInputElement; | ||
| const button = screen.getByRole('button', { name: /add/i }); | ||
|
|
||
| const button = screen.getByRole('button', { name: ADD_REGEX }); | ||
| const form = button.closest('form') as HTMLFormElement; | ||
|
|
||
| fireEvent.change(input, { target: { value: 'New todo' } }); | ||
| fireEvent.click(button); | ||
| fireEvent.submit(form); | ||
|
|
||
| expect(input.value).toBe(''); | ||
| }); | ||
|
|
||
| it('does not call onAdd with empty text', () => { | ||
| const mockOnAdd = vi.fn(); | ||
| render(<AddTodo onAdd={mockOnAdd} />); | ||
|
|
||
| const button = screen.getByRole('button', { name: /add/i }); | ||
| fireEvent.click(button); | ||
|
|
||
| renderWithRouter(<AddTodo onAdd={mockOnAdd} />); | ||
|
|
||
| const button = screen.getByRole('button', { name: ADD_REGEX }); | ||
| const form = button.closest('form') as HTMLFormElement; | ||
| fireEvent.submit(form); | ||
|
|
||
| expect(mockOnAdd).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('trims whitespace from input', () => { | ||
| const mockOnAdd = vi.fn(); | ||
| render(<AddTodo onAdd={mockOnAdd} />); | ||
| renderWithRouter(<AddTodo onAdd={mockOnAdd} />); | ||
|
|
||
| const input = screen.getByPlaceholderText('Add a new todo...'); | ||
| const button = screen.getByRole('button', { name: /add/i }); | ||
|
|
||
| const button = screen.getByRole('button', { name: ADD_REGEX }); | ||
| const form = button.closest('form') as HTMLFormElement; | ||
|
|
||
| fireEvent.change(input, { target: { value: ' New todo ' } }); | ||
| fireEvent.click(button); | ||
| fireEvent.submit(form); | ||
|
|
||
| expect(mockOnAdd).toHaveBeenCalledWith('New todo'); | ||
| }); | ||
| }); | ||
|
|
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,12 @@ | ||||||||||
| import { z } from 'zod'; | ||||||||||
| import { zodResolver } from '@hookform/resolvers/zod'; | ||||||||||
| import { RemixFormProvider, useRemixForm } from 'remix-hook-form'; | ||||||||||
| import { z } from 'zod'; | ||||||||||
| import { Plus } from 'lucide-react'; | ||||||||||
| import { TextField, FormError } from '@lambdacurry/forms'; | ||||||||||
| import { Button } from '@lambdacurry/forms/ui'; | ||||||||||
|
|
||||||||||
| const addTodoSchema = z.object({ | ||||||||||
| text: z.string().min(1, 'Todo text is required').trim(), | ||||||||||
| text: z.string().min(1, 'Todo text is required').trim() | ||||||||||
| }); | ||||||||||
|
Comment on lines
+9
to
10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix validation order: trim before min As written, whitespace-only input passes - text: z.string().min(1, 'Todo text is required').trim()
+ text: z.string().trim().min(1, 'Todo text is required')📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| type AddTodoFormData = z.infer<typeof addTodoSchema>; | ||||||||||
|
|
@@ -20,29 +20,26 @@ export function AddTodo({ onAdd }: AddTodoProps) { | |||||||||
| resolver: zodResolver(addTodoSchema), | ||||||||||
| defaultValues: { text: '' }, | ||||||||||
| submitHandlers: { | ||||||||||
| onValid: (data) => { | ||||||||||
| onValid: data => { | ||||||||||
| // Since we're not using a database we're catching this early and not actually submitting the form to a server | ||||||||||
| onAdd(data.text); | ||||||||||
| methods.reset(); | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| } | ||||||||||
| } | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| return ( | ||||||||||
| <RemixFormProvider {...methods}> | ||||||||||
| <form onSubmit={methods.handleSubmit} className="flex gap-2"> | ||||||||||
| <form className="flex gap-2" onSubmit={methods.handleSubmit}> | ||||||||||
| <div className="flex-1"> | ||||||||||
| <TextField | ||||||||||
| name="text" | ||||||||||
| placeholder="Add a new todo..." | ||||||||||
| className="w-full" | ||||||||||
| /> | ||||||||||
| <TextField name="text" placeholder="Add a new todo..." className="w-full" /> | ||||||||||
| </div> | ||||||||||
| <Button type="submit"> | ||||||||||
| <Plus className="h-4 w-4 mr-2" /> | ||||||||||
| Add | ||||||||||
| </Button> | ||||||||||
| </form> | ||||||||||
| <FormError /> | ||||||||||
| <FormError name="_form" /> | ||||||||||
| </RemixFormProvider> | ||||||||||
| ); | ||||||||||
| } | ||||||||||
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.
Cache key likely wrong: Bun’s lockfile is “bun.lockb”, not “bun.lock”.
Using bun.lock here will cause cache misses. Switch to bun.lockb (or hash both for safety).
Optional: hash both
📝 Committable suggestion
🤖 Prompt for AI Agents