Files
Ready2Blob/.planning/phases/05-tech-debt/05-RESEARCH.md
T
2026-03-30 09:18:44 +02:00

25 KiB

Phase 5: Tech Debt - Research

Researched: 2026-03-30 Domain: React wizard refactoring — conditional rendering, registry-driven UI, test quality Confidence: HIGH

<user_constraints>

User Constraints (from CONTEXT.md)

Locked Decisions

scriptTargets filtering (TECH-01)

  • ReviewStep conditionally renders output blocks based on state.deployment.scriptTargets — blocks are unmounted entirely when their target is deselected (not CSS-hidden)
  • Intune maps to: intuneInstall + intuneDetection blocks
  • RMM maps to: rmmScript block
  • rclone.conf OutputBlock is always shown regardless of scriptTargets
  • ZIP bundle also respects scriptTargets — only files for selected targets are included (not always 4)
  • Edge case: if both targets are deselected, only rclone.conf is shown and ZIP contains only rclone.conf (no blocking or warning needed)

Back button (TECH-02)

  • Must preserve all form data (wizard state is in the store — navigation does not clear it)
  • Back dispatches SET_STEP(2) to return to DeploymentStep

Registry structure (TECH-03)

  • Enrich BACKEND_REGISTRY to Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }>
  • All consumers of BACKEND_REGISTRY[type] that access field arrays must update to BACKEND_REGISTRY[type].fields
  • BackendSelectionStep derives the card list from Object.entries(BACKEND_REGISTRY) — no hardcoded BACKENDS array
  • Display metadata stays in the registry entry so Phase 6 additions auto-surface with zero additional code
  • Existing test assertions on card text ('Azure Blob Storage', etc.) remain valid — source of truth just moves

Dead export removal (TECH-04)

  • Remove BackendFormValues<T> export from src/schemas/index.ts (lines 27-28)
  • Verify no TypeScript errors arise after removal (no consumers expected)

act() warnings (TECH-05)

  • Claude's Discretion — fix the act() warnings in BackendSelectionStep.test.tsx using appropriate testing-library patterns

Claude's Discretion

  • Back button (TECH-02): placement and styling left to planner
  • act() warnings (TECH-05): fix approach left to planner

Deferred Ideas (OUT OF SCOPE)

None — discussion stayed within phase scope. </user_constraints>

<phase_requirements>

Phase Requirements

ID Description Research Support
TECH-01 User sees only the script output blocks matching their selected deployment targets (Intune and/or RMM) in ReviewStep Conditional render pattern; ZIP filter array construction; existing state.deployment.scriptTargets already tracks selections
TECH-02 User can navigate back from ReviewStep using an explicit Back button SET_STEP(2) action exists; DeploymentStep Back button is the established style pattern
TECH-03 Backend list in BackendSelectionStep is automatically derived from BACKEND_REGISTRY keys (no hardcoded list) Registry shape change; three consumers need .fields access update; BackendSelectionStep replaces BACKENDS constant
TECH-04 Dead BackendFormValues<T> export is removed from src/schemas/index.ts Lines 27-28 confirmed; no consumers found via code audit
TECH-05 BackendSelectionStep test suite runs without act() warnings Warnings confirmed in live test run; root cause is fireEvent triggering async state updates without act() wrapping
</phase_requirements>

Summary

This phase is purely internal code quality work — no new user-facing features except the Back button on ReviewStep and the scriptTargets filtering. All changes are surgical: modify four files (registry.ts, schemas/index.ts, BackendSelectionStep.tsx, ReviewStep.tsx) and update three test files to keep assertions aligned.

The codebase is in clean, idiomatic React/TypeScript with Vitest + Testing Library. Patterns are already established: conditional render (not CSS-hide) for content that has no preserved state, store dispatch for navigation, registry-driven loops for field rendering. Phase 5 applies these patterns to the remaining places that weren't wired up in v1.0.

The act() warnings are real and confirmed to be in both BackendSelectionStep.test.tsx (warnings about BackendSelectionStep and WizardProvider components) and ReviewStep.test.tsx (warnings about OutputBlock). The root cause is fireEvent calling React Hook Form's handleSubmit, which triggers async state updates and setTimeout (in OutputBlock.handleCopy) without act() wrapping. The fix is userEvent (which wraps in act()) or explicit act() calls around async sequences.

Primary recommendation: Address each requirement as a focused, independent task. TECH-03 (registry enrichment) has the most ripple effect — do it first so TECH-01 and the BackendSelectionStep work have the final registry shape available.

Standard Stack

Core (already in use — no new installs)

Library Version Purpose Why Standard
React 18.3.1 UI rendering, conditional render, hooks Project baseline
TypeScript 5.5.3 Type safety for registry shape change Project baseline
Vitest 4.1.1 Test runner Project baseline
@testing-library/react 16.3.2 Component testing Project baseline
@testing-library/user-event 14.6.1 User interaction simulation (wraps in act()) Already installed; fixes act() warnings

Installation: No new packages required. @testing-library/user-event is already in devDependencies.

Architecture Patterns

src/
├── schemas/
│   ├── registry.ts          # TECH-03: enrich shape + add displayName/description to each entry
│   └── index.ts             # TECH-03: update .fields access in buildZodSchema; TECH-04: remove BackendFormValues<T>
├── components/wizard/
│   ├── BackendSelectionStep.tsx        # TECH-03: replace BACKENDS const with Object.entries(BACKEND_REGISTRY)
│   ├── BackendSelectionStep.test.tsx   # TECH-05: replace fireEvent with userEvent; TECH-03: verify assertions still pass
│   ├── RemoteConfigStep.tsx            # TECH-03: update BACKEND_REGISTRY[backendType] → BACKEND_REGISTRY[backendType].fields
│   └── ReviewStep.tsx                  # TECH-01: conditional renders; TECH-02: Back button
└── (no new files needed)

Pattern 1: Conditional Render by scriptTargets (TECH-01)

What: Unmount OutputBlocks whose target is not in state.deployment.scriptTargets When to use: When the content has no local state worth preserving between show/hide Why not CSS-hide: OutputBlock has no internal state that must survive deselection (unlike AzureAuthToggle's field values)

// In ReviewStep — derive booleans from state
const showIntune = state.deployment.scriptTargets.includes('intune');
const showRmm = state.deployment.scriptTargets.includes('rmm');

// Conditional render (NOT CSS hidden)
{showIntune && (
  <OutputBlock label="Intune Install Script" content={intuneInstall} filename="intune-install.ps1" disabled={!acknowledged} />
)}
{showIntune && (
  <OutputBlock label="Intune Detection Script" content={intuneDetection} filename="intune-detection.ps1" disabled={!acknowledged} />
)}
{showRmm && (
  <OutputBlock label="RMM Script" content={rmmScript} filename="rmm-script.ps1" disabled={!acknowledged} />
)}

Pattern 2: ZIP Filter by scriptTargets (TECH-01)

What: Build the files array dynamically before passing to downloadZip()

async function handleDownloadZip() {
  const files: { name: string; content: string }[] = [
    { name: 'rclone.conf', content: rcloneConf }, // always included
  ];
  if (showIntune) {
    files.push({ name: 'intune-install.ps1', content: intuneInstall });
    files.push({ name: 'intune-detection.ps1', content: intuneDetection });
  }
  if (showRmm) {
    files.push({ name: 'rmm-script.ps1', content: rmmScript });
  }
  await downloadZip(files, 'rclone-deployment.zip');
}

Pattern 3: Back Button in ReviewStep (TECH-02)

What: Follow DeploymentStep's exact button layout — Back + action button side-by-side at the bottom Established reference: DeploymentStep.tsx lines 90-105 — flex gap-3 mt-6 container, px-4 py-2 text-sm border border-gray-300 rounded-md for Back, blue variant for primary

// Bottom of ReviewStep JSX, before closing </div>
<div className="flex gap-3 mt-6">
  <button
    type="button"
    onClick={() => dispatch({ type: 'SET_STEP', payload: 2 })}
    className="px-4 py-2 text-sm border border-gray-300 rounded-md hover:bg-gray-50"
  >
    Back
  </button>
</div>

Note: dispatch is not currently destructured from useWizard() in ReviewStep — must add it alongside state.

Pattern 4: Registry Shape Enrichment (TECH-03)

What: Change BACKEND_REGISTRY value type from FieldDef[] to { displayName: string; description: string; fields: FieldDef[] }

// registry.ts — new shape
export const BACKEND_REGISTRY: Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }> = {
  azureblob: {
    displayName: 'Azure Blob Storage',
    description: 'Microsoft Azure cloud storage',
    fields: [ /* existing FieldDef array */ ],
  },
  s3: {
    displayName: 'Amazon S3',
    description: 'AWS Simple Storage Service',
    fields: [ /* existing FieldDef array */ ],
  },
  's3-compatible': {
    displayName: 'S3-Compatible',
    description: 'Wasabi, MinIO, Cloudflare R2, and others',
    fields: [ /* existing FieldDef array */ ],
  },
};

Consumers that break and must be updated:

  1. src/schemas/index.ts line 10: const fields = BACKEND_REGISTRY[backendType];const fields = BACKEND_REGISTRY[backendType].fields;
  2. src/components/wizard/RemoteConfigStep.tsx line 58: BACKEND_REGISTRY.azureblob.find(...)BACKEND_REGISTRY.azureblob.fields.find(...)
  3. src/components/wizard/RemoteConfigStep.tsx line 73: BACKEND_REGISTRY[backendType].map(...)BACKEND_REGISTRY[backendType].fields.map(...)

BackendSelectionStep replacement:

// Replace hardcoded BACKENDS const entirely
// Object.entries preserves insertion order in V8 (azureblob first, consistent with WIZD-01 test)
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
  <BackendCard
    key={type}
    name={entry.displayName}
    description={entry.description}
    selected={state.remote.backendType === type}
    onClick={() => handleCardClick(type as BackendType)}
  />
))}

Pattern 5: Fixing act() Warnings (TECH-05)

Root cause confirmed in live test run:

  • BackendSelectionStep.test.tsx: fireEvent.click(azureButton) triggers handleSubmit → async form validation → dispatch in store → state update in WizardProvider / BackendSelectionStep without act() wrapping
  • ReviewStep.test.tsx: fireEvent.click(copyButtons[0]) triggers handleCopysetCopied(true)setTimeout(() => setCopied(false), 2000) — the timeout fires after test assertion, causing the OutputBlock warning

Fix strategy for BackendSelectionStep.test.tsx: Replace fireEvent with userEvent from @testing-library/user-event. userEvent.setup() + await user.click() wraps all state updates in act() automatically.

// Before (produces act() warnings)
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
fireEvent.click(azureButton);

// After (no act() warnings)
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

const user = userEvent.setup();
// ...
await user.click(azureButton);

Fix strategy for ReviewStep act() warnings (OutputBlock setTimeout): Two options (both valid, Claude's Discretion):

  1. Use vi.useFakeTimers() in beforeEach of the clipboard copy tests so setTimeout never fires unexpectedly
  2. Use userEvent for click interactions (same as BackendSelectionStep fix)

Option 1 (fake timers) is more surgical for the OutputBlock case since the setTimeout is a 2-second "Copied!" display reset — it's not a test concern.

Anti-Patterns to Avoid

  • CSS-hiding OutputBlocks: Tempting to add hidden class based on scriptTargets, but decision is locked to unmount — conditional render is the correct approach.
  • Hardcoding the BACKENDS array after registry enrichment: The whole point of TECH-03 is Object.entries(BACKEND_REGISTRY) — do not leave a parallel BACKENDS constant.
  • Wrapping every fireEvent in act(): The correct fix is userEvent, not wrapping fireEvent calls manually — manual act() wrapping produces verbose, harder-to-maintain tests.
  • Removing useMemo calls for conditionally rendered content: intuneInstall, intuneDetection, rmmScript useMemo calls can stay even if the block is hidden — the computation is cheap and the memoized value is used in the ZIP handler too.

Don't Hand-Roll

Problem Don't Build Use Instead Why
Act()-safe user interactions Manual act() wrappers around fireEvent userEvent from @testing-library/user-event userEvent simulates real browser events and automatically wraps in act(); already installed
Fake timers for setTimeout Custom timer management vi.useFakeTimers() / vi.useRealTimers() Vitest built-in; handles all setTimeout/setInterval cleanly
Registry-driven UI ordering Custom sort/filter logic Object.entries() (V8 insertion order) Object.entries() on a Record<BackendType, ...> preserves declaration order — Azure first matches WIZD-01 requirement

Common Pitfalls

Pitfall 1: Forgetting dispatch in ReviewStep

What goes wrong: ReviewStep currently only destructures state from useWizard(). Adding a Back button requires dispatch too. Easy to miss. Why it happens: ReviewStep was read-only in v1.0 — no navigation actions needed. How to avoid: Change const { state } = useWizard(); to const { state, dispatch } = useWizard(); at the top of ReviewStep.

Pitfall 2: registry.test.ts breaks after TECH-03

What goes wrong: registry.test.ts accesses BACKEND_REGISTRY[backend] and calls .length and .find() directly on the registry entry — these will fail after the shape change to { displayName, description, fields }. Lines affected: Lines 14 (BACKEND_REGISTRY[backend].length), 19 (for (const field of BACKEND_REGISTRY[backend])), 37/43/50 (BACKEND_REGISTRY.azureblob.find(...), etc.) How to avoid: Update registry.test.ts to access .fields — e.g., BACKEND_REGISTRY[backend].fields.length, BACKEND_REGISTRY[backend].fields.find(...).

Pitfall 3: ReviewStep.test.tsx asserts exactly 4 OutputBlocks / 4 downloads

What goes wrong: Tests DOWN-02, DOWN-03, DOWN-04, DOWN-05 use positional indexing (downloadButtons[1], downloadButtons[2], downloadButtons[3]) and expect(files).toHaveLength(4). After TECH-01, when scriptTargets is both (default), 4 blocks still render — but tests for individual downloads will break if called with scriptTargets that hide blocks. Current status: Default INITIAL_STATE has scriptTargets: ['intune', 'rmm'] so existing tests still pass with all 4 blocks. Tests specifically for filtered rendering are new (Wave 0 stubs needed). How to avoid: Existing ReviewStep tests can remain as-is (they use default state = both targets). Add new test cases for filtered scenarios (intune-only, rmm-only, neither) — these are new Wave 0 stubs.

Pitfall 4: BackendSelectionStep.test.tsx assertions on card text remain valid

What goes wrong: After TECH-03, tests check for 'Azure Blob Storage', 'Amazon S3', 'S3-Compatible' text. These strings move from the hardcoded BACKENDS array to BACKEND_REGISTRY[type].displayName. As long as displayName values match, tests pass without any change. How to avoid: Use the exact same display strings when adding displayName to registry entries.

Pitfall 5: TypeScript error after removing BackendFormValues

What goes wrong: If any file imports BackendFormValues from src/schemas/index.ts, removing it causes a TS compile error. Evidence: Code audit found no consumers — BackendFormValues only appears in src/schemas/index.ts lines 27-28. But TypeScript compile check is the definitive verification step. How to avoid: After removal, run tsc --noEmit or npm run build to confirm zero errors.

Pitfall 6: userEvent requires setup() call

What goes wrong: userEvent.click() was called directly in older versions of @testing-library/user-event. In v14 (installed: 14.6.1), the correct API is const user = userEvent.setup() then await user.click(). How to avoid: Always use userEvent.setup() pattern — not the legacy userEvent.click() shorthand.

Code Examples

Verified pattern: userEvent v14 API

// Source: @testing-library/user-event v14 official API
import userEvent from '@testing-library/user-event';

describe('BackendSelectionStep', () => {
  it('clicking a backend card dispatches SET_BACKEND_TYPE and SET_STEP', async () => {
    const user = userEvent.setup();
    renderStep();
    const nameInput = screen.getByRole('textbox');
    await user.type(nameInput, 'my-remote');
    const azureButton = screen.getAllByRole('button').find(b => b.textContent?.includes('Azure Blob Storage'))!;
    await user.click(azureButton);
    expect(screen.queryByRole('alert')).toBeNull();
  });
});

Verified pattern: vi.useFakeTimers() for setTimeout-based act() warnings

// Source: Vitest official docs — timer mocking
import { beforeEach, afterEach, vi } from 'vitest';

beforeEach(() => {
  vi.useFakeTimers();
});

afterEach(() => {
  vi.useRealTimers();
});

it('clicking Copy calls clipboard.writeText', async () => {
  renderStep();
  const checkbox = screen.getByRole('checkbox');
  fireEvent.click(checkbox);
  const copyButtons = screen.getAllByText('Copy');
  fireEvent.click(copyButtons[0]);
  expect(navigator.clipboard.writeText).toHaveBeenCalledTimes(1);
  // setTimeout(() => setCopied(false)) never fires during test — no act() warning
});

Verified pattern: Object.entries() on enriched registry

// Source: MDN Object.entries() — insertion order preserved for string keys
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
  <BackendCard
    key={type}
    name={entry.displayName}
    description={entry.description}
    selected={state.remote.backendType === type}
    onClick={() => handleCardClick(type as BackendType)}
  />
))}

State of the Art

Old Approach Current Approach When Changed Impact
fireEvent for all test interactions userEvent.setup() + await user.click() @testing-library/user-event v14 Eliminates act() warnings from async state updates triggered by simulated events
BACKEND_REGISTRY: Record<BackendType, FieldDef[]> Record<BackendType, { displayName: string; description: string; fields: FieldDef[] }> Phase 5 (TECH-03) BackendSelectionStep no longer needs a parallel BACKENDS array; Phase 6 additions auto-surface

Deprecated/outdated:

  • fireEvent for click interactions that trigger state updates: still works but causes act() warnings; replace with userEvent where state updates occur
  • Hardcoded BACKENDS array in BackendSelectionStep.tsx: dead after TECH-03; replaced by Object.entries(BACKEND_REGISTRY)
  • BackendFormValues<T> type export: dead code — no consumers exist; removed in TECH-04

Open Questions

  1. ReviewStep.test.tsx: DOWN-05 asserts files.toHaveLength(4) — does this test need updating?

    • What we know: Default INITIAL_STATE has scriptTargets = ['intune', 'rmm'], so with default state the ZIP still gets 4 files.
    • What's unclear: Should the TECH-01 implementation also add new test cases covering filtered scenarios (2 files for intune-only, etc.)?
    • Recommendation: Keep the existing DOWN-05 test intact (it documents correct behavior when both targets are selected). Add new test cases in Wave 0 for filtered scenarios (intune-only, rmm-only, neither).
  2. act() warnings in ReviewStep.test.tsx — fix in this phase or leave?

    • What we know: TECH-05 requirement text says "BackendSelectionStep test suite" specifically. ReviewStep warnings are from OutputBlock's setTimeout.
    • What's unclear: Whether the phase goal implicitly includes ReviewStep warnings given CONTEXT.md says "eliminate act() warnings from the BackendSelectionStep test suite."
    • Recommendation: TECH-05 scope is BackendSelectionStep. Fix ReviewStep act() warnings as a bonus in the same task (low effort — add vi.useFakeTimers() to the relevant ReviewStep tests). If time-constrained, BackendSelectionStep warnings are the acceptance criterion.

Validation Architecture

Test Framework

Property Value
Framework Vitest 4.1.1
Config file vitest.config.ts (root)
Quick run command npx vitest run src/components/wizard/BackendSelectionStep.test.tsx src/components/wizard/ReviewStep.test.tsx src/schemas/registry.test.ts
Full suite command npx vitest run

Phase Requirements → Test Map

Req ID Behavior Test Type Automated Command File Exists?
TECH-01 Intune blocks absent when scriptTargets excludes 'intune' unit npx vitest run src/components/wizard/ReviewStep.test.tsx Wave 0 (new cases needed)
TECH-01 RMM block absent when scriptTargets excludes 'rmm' unit npx vitest run src/components/wizard/ReviewStep.test.tsx Wave 0 (new cases needed)
TECH-01 ZIP contains only rclone.conf when both targets deselected unit npx vitest run src/components/wizard/ReviewStep.test.tsx Wave 0 (new cases needed)
TECH-02 Back button dispatches SET_STEP(2) unit npx vitest run src/components/wizard/ReviewStep.test.tsx Wave 0 (new case needed)
TECH-03 BackendSelectionStep renders cards from BACKEND_REGISTRY unit npx vitest run src/components/wizard/BackendSelectionStep.test.tsx (existing assertions still valid)
TECH-03 registry.test.ts field access via .fields unit npx vitest run src/schemas/registry.test.ts Wave 0 (update existing)
TECH-04 No TypeScript error after BackendFormValues removal type-check npx tsc --noEmit (compile check)
TECH-05 BackendSelectionStep test suite zero act() warnings unit npx vitest run src/components/wizard/BackendSelectionStep.test.tsx 2>&1 | grep -c "act(" (existing tests, modified)

Sampling Rate

  • Per task commit: npx vitest run (full suite — only 3.4s, fast enough for every commit)
  • Per wave merge: npx vitest run + npx tsc --noEmit
  • Phase gate: Full suite green + zero TypeScript errors before /gsd:verify-work

Wave 0 Gaps

  • src/components/wizard/ReviewStep.test.tsx — add cases for TECH-01: intune-only, rmm-only, neither (requires WizardProvider accepting initial state, or dispatching SET_DEPLOYMENT in test setup)
  • src/components/wizard/ReviewStep.test.tsx — add case for TECH-02: Back button renders and dispatches SET_STEP(2)
  • src/schemas/registry.test.ts — update field access to .fields after TECH-03 shape change (existing tests break without this update)

Note on ReviewStep test state injection: Current renderStep() wraps with bare WizardProvider which uses INITIAL_STATE. To test filtered rendering, tests need a way to render with custom deployment state. Options: (a) export a WizardProvider variant accepting initialState prop, or (b) render and dispatch SET_DEPLOYMENT in test setup before assertions. Option (b) requires no code changes to the provider — preferred.

Sources

Primary (HIGH confidence)

  • Direct code audit of src/components/wizard/ReviewStep.tsx — confirmed no dispatch, no Back button, always 4 OutputBlocks
  • Direct code audit of src/components/wizard/BackendSelectionStep.tsx — confirmed hardcoded BACKENDS array
  • Direct code audit of src/schemas/registry.ts — confirmed current shape is Record<BackendType, FieldDef[]>
  • Direct code audit of src/schemas/index.ts — confirmed BackendFormValues at lines 27-28 with no consumers
  • Live test run (npx vitest run) — confirmed act() warnings present in both BackendSelectionStep and ReviewStep test files, all 98 tests passing
  • src/store/types.ts — confirmed SET_STEP action exists, scriptTargets in WizardState.deployment
  • package.json — confirmed @testing-library/user-event 14.6.1 already installed

Secondary (MEDIUM confidence)

  • @testing-library/user-event v14 API: userEvent.setup() pattern verified against installed version and established community usage

Tertiary (LOW confidence)

  • None

Metadata

Confidence breakdown:

  • Standard stack: HIGH — all libraries already in use, confirmed from package.json
  • Architecture: HIGH — all patterns derived from direct code audit of existing files
  • Pitfalls: HIGH — most pitfalls discovered by cross-referencing actual test files with planned changes
  • act() fix: HIGH — userEvent.setup() pattern confirmed against installed v14.6.1

Research date: 2026-03-30 Valid until: 2026-05-30 (stable libraries — React 18, Vitest 4, userEvent 14)