fix(contracts): infer V from keyof S & string in parseContract to fix TS2345 in 15 edge functions#115
Conversation
…ix TS2345 in 15 edge functions Bug: `Edge Functions — Deno typecheck` falhava em 15/81 edge functions desde o merge do PR #87 (feat(contracts): migrate 13 edge functions to parseContract). Funções afetadas: bi-copilot, block-ip-temporarily, e2e-cleanup, force-global-logout, kit-ai-builder, market-intelligence-insights, ownership-audit, ownership-repair, product-webhook, send-transactional-email, simulation-orchestrator, step-up-verify, sync-external-db, trends-insights, webhook-dispatcher. Causa-raiz: A signature anterior era: parseContract<V extends string, S extends Record<V, z.ZodTypeAny>>( req, schemas: ContractSchemas<V> & { versions: S }, opts ) TypeScript inferia V somente de `defaultVersion: "1" as const` (literal "1"). ContractSchemas<V> exigia `versions: Record<V, z.ZodTypeAny>` = `Record<"1", ...>`. Mas `versions: { "1": ..., "2": ... }` tem mais chaves que `Record<"1", ...>`. A intersecção `& { versions: S }` produzia `versions["1"]` com tipo `ZodTypeAny & ZodObject<...>` — quando `.deepPartial()` era invocado, o tipo retornado era incompatível (ZodOptional<ZodString> vs ZodString esperado), gerando TS2345 em cada call de parseContract(). Fix: Inverte a ordem de inferência. Agora `S extends Record<string, z.ZodTypeAny>` é o generic primário (sem restrição prévia de V), e V é derivado de S via `keyof S & string`. Isso permite que TypeScript infira corretamente todas as chaves de versions, e o tipo de `schema.versions["X"]` é o ZodObject concreto (não a intersecção problemática). ContractSchemas também ganhou um segundo generic S (com default = Record<V, ...>) para suportar a nova signature mantendo compat com chamadores que usam ContractSchemas<V> em 1 arg (S vira o default). Lab-validado: - node scripts/typecheck-edge-functions.mjs → 81/81 ✅ (antes: 15/81 ❌) - npx vitest run tests/contracts/ → 92/92 testes passam ✅ - Runtime: zero mudanças (só types) - Pesquisa no codebase: ContractSchemas só é importado para re-export em _shared/contracts/index.ts; nenhum schema usa `: ContractSchemas<V>` como anotação. Mudança 100% backward-compat para chamadores existentes. Causa do PR #87 ter passado e o main estar vermelho agora: Deno 2.x faz auto-update do toolchain; entre o merge (06:42 -0300) e os runs subsequentes, a versão de TypeScript embarcada mudou ou a resolução de tipos do esm.sh ficou mais estrita. O diff em si nunca foi compatível com a inferência TypeScript 6.x — só passou por sorte de cache no PR #87. Plano "10/10": este é o Bug #2 de 4. Próximos: #3 (Test Coverage), #4 (quality Run tests).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA PR refatora a parametrização de genéricos em ChangesRefatoração de Genéricos em parseContract
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutos 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes TypeScript inference issues in the shared Edge Function contract parser by changing parseContract to infer the supported version union (V) from the keys of the provided versions object, resolving TS2345 typecheck failures in multiple Edge Functions without changing runtime behavior.
Changes:
- Extend
ContractSchemasto parameterizeversionsas a genericS(defaulting toRecord<V, ZodTypeAny>) instead of forcingRecord<V, ...>. - Update
parseContractto inferSfirst and deriveVaskeyof S & string, avoiding problematic intersections onversions["1"]. - Adjust
parseContractreturn type accordingly, preserving existing runtime logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ctedRoute redirect Bug #3 do plano 10/10. O job CI "Test Coverage" (vitest run --coverage) falhava em main com 1 test failing em 1845 total. Causa-raiz: ProtectedRoute.tsx:37 redireciona para /auth (canonico em prod): if (!user) return <Navigate to="/auth" .../>; Mas os testes admin/reduced-app-navigation.test.tsx e admin/route-no-error-element.test.tsx configuravam: <Route path="/login" element={<LoginStub />} /> Logo o redirect cai num path sem element matching → renderiza vazio → screen.getByTestId("page-login") falha com: Unable to find an element by: [data-testid="page-login"] <body><div /></body> Em produção src/routes/public-routes.tsx mantem AMBAS as rotas: <Route path="/auth" element={<Auth />} /> <Route path="/login" element={<Auth />} /> (alias) Mas TODOS os 3 redirects de guards (ProtectedRoute, AdminRoute, DevRoute) apontam para /auth, que é a rota canonica. /login eh apenas um alias defensivo. Fix cirurgico: mudar path="/login" -> path="/auth" nos 2 testes para alinhar com o destino real do Navigate. Por que nao mudar o redirect para /login no codigo de producao? - Risco alto: OAuth callbacks usam /auth como retorno canonico - 3 arquivos de prod afetados vs 2 de teste Por que nao adicionar AMBAS as rotas no teste? - Mais verbose; o teste deve refletir a realidade da prod - /login alias soh existe pra defensive coding em prod, nao deveria interessar pra um teste de redirect Validacao em lab (clone fresco do main + npm ci + node v22): Antes: tests/admin/route-no-error-element.test.tsx > "anônimo em rota protegida → /login — árvore limpa" -> FAIL (5s timeout em getByTestId) Depois: tests/admin/reduced-app-navigation.test.tsx (6 tests) ✅ tests/admin/route-no-error-element.test.tsx (7 tests) ✅ Subset broader: 314/314 tests passing em 21 files (admin/ + contracts/ + cloud-status + price-freshness) Em produção: zero mudancas. Apenas testes alinhados com Navigate. Roadmap "10/10": ✅ Bug #1 Migrations sync guard (PR #111, 5f3ec9d) ✅ Bug #2 Edge Deno typecheck (PR #115, 0c650ca) 🟢 Bug #3 Test Coverage (este PR) 🟡 Bug #4 ESLint baseline gate (proximo - refinado: nao eh "Run tests" como pensavamos, eh ESLint regression)
) * fix(tests): align /login → /auth in route guards tests to match ProtectedRoute redirect Bug #3 do plano 10/10. O job CI "Test Coverage" (vitest run --coverage) falhava em main com 1 test failing em 1845 total. Causa-raiz: ProtectedRoute.tsx:37 redireciona para /auth (canonico em prod): if (!user) return <Navigate to="/auth" .../>; Mas os testes admin/reduced-app-navigation.test.tsx e admin/route-no-error-element.test.tsx configuravam: <Route path="/login" element={<LoginStub />} /> Logo o redirect cai num path sem element matching → renderiza vazio → screen.getByTestId("page-login") falha com: Unable to find an element by: [data-testid="page-login"] <body><div /></body> Em produção src/routes/public-routes.tsx mantem AMBAS as rotas: <Route path="/auth" element={<Auth />} /> <Route path="/login" element={<Auth />} /> (alias) Mas TODOS os 3 redirects de guards (ProtectedRoute, AdminRoute, DevRoute) apontam para /auth, que é a rota canonica. /login eh apenas um alias defensivo. Fix cirurgico: mudar path="/login" -> path="/auth" nos 2 testes para alinhar com o destino real do Navigate. Por que nao mudar o redirect para /login no codigo de producao? - Risco alto: OAuth callbacks usam /auth como retorno canonico - 3 arquivos de prod afetados vs 2 de teste Por que nao adicionar AMBAS as rotas no teste? - Mais verbose; o teste deve refletir a realidade da prod - /login alias soh existe pra defensive coding em prod, nao deveria interessar pra um teste de redirect Validacao em lab (clone fresco do main + npm ci + node v22): Antes: tests/admin/route-no-error-element.test.tsx > "anônimo em rota protegida → /login — árvore limpa" -> FAIL (5s timeout em getByTestId) Depois: tests/admin/reduced-app-navigation.test.tsx (6 tests) ✅ tests/admin/route-no-error-element.test.tsx (7 tests) ✅ Subset broader: 314/314 tests passing em 21 files (admin/ + contracts/ + cloud-status + price-freshness) Em produção: zero mudancas. Apenas testes alinhados com Navigate. Roadmap "10/10": ✅ Bug #1 Migrations sync guard (PR #111, 5f3ec9d) ✅ Bug #2 Edge Deno typecheck (PR #115, 0c650ca) 🟢 Bug #3 Test Coverage (este PR) 🟡 Bug #4 ESLint baseline gate (proximo - refinado: nao eh "Run tests" como pensavamos, eh ESLint regression) * fix(tests): align /login → /auth in route-no-error-element test Parte 2/2 do Bug #3 do plano 10/10 (mesmo PR). Mesma causa-raiz que reduced-app-navigation.test.tsx: o teste configurava <Route path="/login" element={<LoginStub />} /> mas o ProtectedRoute.tsx:37 redireciona para /auth. Test afetado: "anônimo em rota protegida → /login — árvore limpa" (linha 317) Antes: getByTestId("page-login") timeout 5s (body vazio porque /auth não tinha route matching no MemoryRouter do test) Depois: ✅ 7/7 tests passing Diff: 1 linha (path="/login" → path="/auth").
🐛 Bug
Job
Edge Functions — Deno typecheckfalha em 15 de 81 edge functions desde o merge do PR #87 (feat(contracts): migrate 13 edge functions to parseContract).Funções afetadas:
bi-copilot,block-ip-temporarily,e2e-cleanup,force-global-logout,kit-ai-builder,market-intelligence-insights,ownership-audit,ownership-repair,product-webhook,send-transactional-email,simulation-orchestrator,step-up-verify,sync-external-db,trends-insights,webhook-dispatcher.Erro típico (em cada uma):
🔍 Causa-raiz
A signature anterior era:
Cadeia de inferência problemática:
defaultVersion: "1" as const→ TypeScript infereV = "1"(literal)ContractSchemas<V>exigeversions: Record<V, z.ZodTypeAny>=Record<"1", ...>versions: { "1": ..., "2": ... }tem mais chaves que oRecord<"1", ...>exigido& { versions: S }fazversions["1"]virarZodTypeAny & ZodObject<...>.deepPartial()nesse tipo intersectado, devolve um tipo incompatívelparseContract()✅ Fix
Inverte a ordem de inferência:
Sé o generic primário, eVé derivado deSviakeyof S & string:Por quê funciona: agora
Sé inferido livremente das chaves deversions(sem restrição prévia), eVé derivado.schema.versions["X"]passa a ser oZodObjectconcreto (não a intersecção problemática).defaultVersioncontinua sendo tipado comoV(qualquer chave válida de S).🧪 Validação em lab
Container Linux limpo, Deno 2.8.0 (TypeScript 6.0.3), cache zerado:
node scripts/typecheck-edge-functions.mjsnpx vitest run tests/contracts/(92 testes)deno check supabase/functions/trends-insights/index.ts(isolado)🛡️ Análise de compatibilidade
ContractSchemasagora aceita 2 generics, mas o segundo (S) tem default= Record<V, z.ZodTypeAny>→ chamadores que usamContractSchemas<V>em 1 arg continuam funcionando.ContractSchemassó aparece em 2 lugares — sua definição emparse.tse o re-export em_shared/contracts/index.ts. Nenhum schema usa como anotação (todos usam inferência). Zero risco de quebrar terceiros.🤔 Por quê só agora?
PR #87 mergeou hoje cedo (06:42 -0300) com CI verde. O job ficou vermelho depois sem ninguém tocar nesses arquivos. Hipótese: Deno 2.x faz auto-update do toolchain TypeScript embarcado. Entre o merge e os runs subsequentes, a versão do TS ficou mais estrita na resolução de generics, expondo o bug que sempre esteve lá. O fix em si nunca foi compatível com a inferência rigorosa — só passou por sorte/cache.
🛣️ Plano "10/10" — progresso
🤖 PR proposto pelo Claude (Agente BPM/Tech Lead). Lab-test completo: 81/81 edge functions + 92/92 contract tests passam.
Summary by cubic
Fixes TS2345 type errors in 15 edge functions by changing
parseContractto infer versionVfromkeyof S & string. Restores Deno typecheck to 81/81 with no runtime changes.S extends Record<string, z.ZodTypeAny>the primary generic and deriveVaskeyof S & string.ContractSchemasto acceptVandS(with defaults) and setversions: S; adjustparseContractsignature and return type accordingly.Written for commit b4c08b3. Summary will update on new commits. Review in cubic
Summary by CodeRabbit