Uh oh!
There was an error while loading. Please reload this page.
test: add @ybyra/react unit tests, fix playground tests, and fix docs MIME error - #2
Conversation
📝 WalkthroughWalkthroughThis PR introduces comprehensive test coverage for React utilities (icons, proxy, registry, translation, and validation), updates documentation navigation to external URLs, expands test mocking infrastructure across projects, and adjusts TypeScript and Vitest configurations to accommodate test files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@playground/sveltekit/tests/src/settings/icons.test.ts`:
- Line 14: Remove the ineffective top-level dynamic import
import('../../../src/lib/settings/icons') — it’s a no-op because it’s neither
awaited nor assigned; instead rely on the awaited import already present inside
the test (the dynamic import used at line 18) or, if you intended module
initialization before tests, replace the bare import with an awaited
import/assignment in a beforeAll hook. Specifically delete the bare import(...)
statement (or convert it to await import('../../../src/lib/settings/icons') in a
setup block) and keep the existing awaited import inside the test.
🧹 Nitpick comments (5)
packages/react/tsconfig.json (1)
11-12: Consider adding.tsxtest file exclusions.The current patterns exclude
.test.tsand.spec.tsfiles but not their.tsxcounterparts. If any React component tests use JSX syntax (.test.tsxor.spec.tsx), they would still be included in the compilation.💡 Proposed fix to cover TSX test files
"include": ["src"], - "exclude": ["src/**/*.test.ts", "src/**/*.spec.ts"]+ "exclude": ["src/**/*.test.ts", "src/**/*.spec.ts", "src/**/*.test.tsx", "src/**/*.spec.tsx"]playground/sveltekit/tests/src/settings/icons.test.ts (1)
4-12: Duplicate mock — already defined in setup.ts.The
lucide-sveltemock is already configured inplayground/sveltekit/tests/setup.ts(which is referenced invitest.config.mtsas a setup file). This inline mock is redundant and could cause confusion about which mock takes precedence.♻️ Proposed fix to remove duplicate mock
import { resolveActionIcon } from '@ybyra/sveltekit' -import { describe, it, expect, vi } from 'vitest'--vi.mock('lucide-svelte', () => ({- Plus: 'Plus',- Eye: 'Eye',- Pencil: 'Pencil',- Save: 'Save',- Send: 'Send',- X: 'X',- Trash2: 'Trash2',-}))+import { describe, it, expect } from 'vitest'packages/react/src/validation.test.ts (3)
5-18: Consider extracting shared test helper.The
makeFieldConfighelper is duplicated fromproxy.test.ts. While duplication in test files is generally acceptable for isolation, if more test files need this helper, consider extracting it to a shared test utilities file.
42-45: Minor: Comment says "returns null" but function returns empty array.The test name says "returns null for non-empty string" but
validateFieldreturns an array. The assertion is correct (toHaveLength(0)), but the test name could be clearer.📝 Suggested test name clarification
- it('returns null for non-empty string', () => {+ it('returns no errors for non-empty string', () => { const errors = validateField('hello', [rule]) expect(errors).toHaveLength(0) })Similar naming appears in other tests (lines 47, 52, 75, 80, 95, 110, 115, 130, 145, 160, 175).
136-164: Consider adding date edge case tests.The
minDateandmaxDatetests use ISO date strings which is good. Consider adding tests for:
- Equal date (boundary condition:
'2024-01-01'with minDate'2024-01-01')- Invalid date strings (to verify graceful handling)
This is optional as the current coverage is adequate for typical use cases.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Description
@ybyra/react, covering all non-hook utility modules — 79 tests across 5 filesplayground/sveltekitandplayground/react-nativeNew tests (
packages/react/src/)validation.test.tsregisterValidator,validateField,validateAllFields, translate passthroughproxy.test.tscreateStateProxychange tracking,createSchemaProxyoverride tracking, immutability guaranteestranslate.test.tsresolveFieldLabel,resolveGroupLabel,resolveActionLabelfallback chainsicons.test.tsconfigureIcons,resolveActionIcon,resolveGroupIcondomain→common fallbackregistry.test.tsregisterRenderers,getRenderer,createRegistryglobal/scoped isolationPlayground fixes
playground/sveltekit— All 14 test suites were failing:esbuild.tsconfigRaw: '{}'in vitest config to bypass missing.svelte-kit/tsconfig.json$app/pathsmock (was aliased for$app/navigationand$app/statebut not$app/paths)Sendicon tolucide-sveltemocks inlayout.test.tsandicons.test.tslucide-sveltemock intests/setup.tsfor coverage of all route testsplayground/react-native— App tests (add, view, edit, index) were failing:@ybyra/persistence/webalias in vitest config (subpath wasn't resolved)vi.mock('@ybyra/persistence/web')inpackages/react-native/testing/setup.tsDocs fix — MIME type error on demo pages
Symptom: Every VitePress documentation page logged console errors:
Root cause: The "Demos" dropdown in the VitePress nav used relative links (
/demo/react-web/, etc.). VitePress only recognizes links as external if they start with a protocol (https://). Relative paths are treated as internal routes, causing VitePress to prefetch non-existent page modules — the server returned the 404 HTML page instead of JavaScript, triggering strict MIME type checking.Fix: Changed all four demo nav links in
docs/.vitepress/config.tsto use absolute URLs (https://devitools.github.io/ybyra/demo/react-web/, etc.). VitePress now correctly treats them as external links and skips prefetching.Why
@ybyra/reactis the most complex package in the monorepo (form state, proxy system, validation, i18n, icon resolution) but had zero tests. These five modules are pure functions with no React dependency, so they can be tested with plain vitest — no jsdom or@testing-library/reactneeded.The playground test failures were pre-existing (not introduced by this PR) but blocked the full
pnpm testsuite from passing.Bug documented
createSchemaProxyinteracts withstructuredCloneupstream, butpattern()inTextFieldDefinitionstores a rawRegExpin validation params.structuredClonethrows onRegExpobjects. This is documented in a test comment inproxy.test.tsfor a future fix in core.Summary by CodeRabbit
Tests
Chores