Skip to content

Commit 7a4b3cc

Browse files
committed
Remove caml-react type widening now that 0.1.0 ships the prop
Verified upstream: @os-legal/caml-react@0.1.0 (the version yarn.lock already resolves) ships dist/index.d.ts with resolveImageSrc on CamlArticleProps, CamlChapterRendererProps, and the block-renderer prop interfaces. The local typecheck failure on main was a stale node_modules on contributors who had not re-installed since the upstream release. Drop the local widening in CamlDirectiveRenderer.tsx; CamlArticle is imported with its real types again. tsc --noEmit passes after a fresh yarn install. Updated the CHANGELOG entry to point at the dependency refresh as the resolution rather than the local workaround. Refs: Open-Source-Legal/caml#8 (closed as completed).
1 parent e396304 commit 7a4b3cc

2 files changed

Lines changed: 2 additions & 17 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3131
- **Centralized extract and user-profile route resolution in `CentralRouteManager`** (`frontend/src/routing/CentralRouteManager.tsx`, `frontend/src/components/routes/ExtractDetailRoute.tsx`, `frontend/src/components/routes/UserProfileRoute.tsx`, new `frontend/src/components/routes/ProfileRedirect.tsx`, `frontend/src/utils/navigationUtils.ts`, `frontend/src/graphql/cache.ts`): `ExtractDetailRoute` and `UserProfileRoute` were each calling `useParams()`, running their own GraphQL resolution queries, and writing entity reactive vars — duplicating the four-phase flow documented in `docs/frontend/routing_system.md` and racing the manager during back-navigation. Both are now thin consumers that read `openedExtract` / `openedUser` / `routeLoading` / `routeError`. `parseRoute` learned `/users/:slug` and `/extracts/:extractId`; the manager added a `GET_USER` lazy query and a Phase 1 user branch alongside the existing extract handling. New `openedUser` reactive var (typed `OpenedUserProfile`) joins the routing-owned set. `/profile` is now served by a small `ProfileRedirect` that uses `backendUserObj` to redirect to `/users/<slug>` — auth-driven, not URL-driven, so legitimately outside the manager. Smaller fixes that ride along: `views/Corpuses.tsx:1807` swaps `window.history.replaceState` for `navigate({ pathname, search }, { replace: true })` so query-param mutations stay inside React Router; redundant `openedLabelset(null)` calls in `LabelSetDetailPage` and `LabelSetLandingRoute` handlers were removed (Phase 1 already clears the var on browse navigation).
3232
- Test coverage: 8 new `parseRoute` tests, 3 new manager tests (user resolve, user not-found, extract by id), updated `beforeEach` to reset `openedExtract` / `openedUser`. New `frontend/src/routing/__tests__/centralRouteDiscipline.test.ts` is a static regression test that grep-walks `frontend/src/` and fails if any production file outside the manager and `cache.ts` SETs one of the 15 routing-owned reactive vars — caught three pre-existing `openedLabelset` writes during development that were also fixed in this PR. New `frontend/tests/e2e/user-and-extract-routes.spec.ts` deep-links `/users/<slug>`, verifies the `/profile` redirect, and exercises the dumb-consumer error path on `/extracts/<unknown-id>`. `tests/e2e/helpers.ts` `VIEWS` catalog now includes `/users/admin` so the existing login-and-navigation walk also covers the user route.
3333
- Doc updates: `docs/frontend/routing_system.md` route-pattern table now lists `/users/:slug`, `/extracts/:extractId`, and `/label_sets/:labelsetId`; the "ONLY CentralRouteManager may SET" list and the four critical RULE blocks were extended to include `openedThread`, `openedLabelset`, and `openedUser`; the new discipline test is referenced as the CI enforcement mechanism.
34-
- **Pre-existing `tsc` failure on `frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx`** resolved as part of the same PR: the installed `@os-legal/caml-react@0.0.1` d.ts files lack the `resolveImageSrc` prop that the declared semver (`^0.1.0`) source already exposes, so `tsc --noEmit` was failing on `main`. Added a narrow local type widening with a comment so `tsc` compiles cleanly until the lockfile is refreshed; runtime behavior is unchanged.
34+
- **Pre-existing `tsc` failure on `frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx`** resolved by refreshing `@os-legal/caml-react` to `0.1.0` (latest on the registry). The lockfile had been pinned to `0.0.1`, whose published `dist/index.d.ts` lacks the `resolveImageSrc` prop that the consumer passes through; `0.1.0` ships the prop on `CamlArticleProps`, `CamlChapterRendererProps`, and the block-renderer prop interfaces. `yarn upgrade @os-legal/caml-react@^0.1.0 @os-legal/caml@^0.1.0` updates the lockfile; no code changes needed.
3535
- **Simplified `RelationGroup.updateForAnnotationDeletion` pruning logic** (Issue #1317, follow-up to #1314, `frontend/src/components/annotator/types/annotations.ts:40-60`): The method previously branched on four near-duplicate conditions (`sourceEmpty && nowTargetEmpty`, `targetEmpty && nowSourceEmpty`, `!sourceEmpty && nowSourceEmpty`, `!targetEmpty && nowTargetEmpty`) each returning `undefined`. All four are equivalent to a single `nowSourceEmpty || nowTargetEmpty` check (`filter` is monotonic, so an originally-empty side stays empty after filtering). Collapsed the branches and removed the now-unused `sourceEmpty` / `targetEmpty` locals per the project's DRY guideline. Behavior is unchanged; existing regression tests in `frontend/src/components/annotator/types/__tests__/annotations.test.ts` still pass unmodified, confirming the simplification is semantics-preserving.
3636
- **Frontend Vitest unit coverage provider switched from V8 to Istanbul** (`frontend/vite.config.ts:210-230`, `frontend/package.json:154`, `frontend/yarn.lock`): The merged `frontend` Codecov flag was landing at ~44% even though `frontend-component` alone was at ~61% on the same code — impossible for a union-aggregated metric unless the three per-suite uploads were measuring on different yardsticks. Root cause: Vitest's `@vitest/coverage-v8` provider emits ~183 `DA:` records per source file (~86k across ~480 files) because V8's native coverage API reports hits for nearly every executable line — imports, declarations, block-closing `}`, etc. — as needed for engine profiling and DevTools. `vite-plugin-istanbul` (used by Playwright CT and E2E suites) emits ~61 `DA:` per file (~27k total) because it only instruments statements. Same code, ~3× denominator mismatch. When Codecov unions multiple uploads under one flag it keeps V8's larger line-number set; Istanbul hits from CT/E2E land on a subset of those line numbers and can never close the gap. Swapped the unit-coverage provider to Istanbul so all three suites report on the same universe. Swapped the `@vitest/coverage-v8` devDep for `@vitest/coverage-istanbul` at the same `^3.1.2` version to match the `vitest` major.minor, changed `coverage.provider` to `"istanbul"` in the Vitest config, regenerated `yarn.lock`, and updated the `all: true` rationale comment to stop singling out V8. The per-suite `flags:` tagging in `frontend.yml` / `frontend-e2e.yml` is unchanged — server-side aggregation still handles the merge. Verified locally: the new unit lcov reports ~451 `SF:` and ~24k `DA:` (previously ~86k), matching the CT/E2E scale. Trade-off: unit-test runtime under coverage grows somewhat (Istanbul transforms source pre-execution), likely 10–30s on this suite. `tsc --noEmit` clean.
3737

frontend/src/components/corpuses/caml/CamlDirectiveRenderer.tsx

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,8 @@
1313
* or any specific directive. All behavior comes from registered handlers.
1414
*/
1515
import React, { useMemo } from "react";
16-
import type { ComponentType } from "react";
1716
import type { CamlDocument, CamlProse } from "@os-legal/caml";
18-
import {
19-
CamlArticle as CamlArticleUntyped,
20-
CamlThemeProvider,
21-
} from "@os-legal/caml-react";
22-
23-
// The @os-legal/caml-react@0.1.x source supports `resolveImageSrc`, but the
24-
// 0.0.1 build still resident in some lockfiles ships d.ts files without it.
25-
// Until the lockfile catches up, widen the prop surface locally so consumer
26-
// code keeps the strong type for the props the published d.ts does declare.
27-
type CamlArticleExtraProps = {
28-
resolveImageSrc?: (src: string) => string | undefined;
29-
};
30-
const CamlArticle = CamlArticleUntyped as ComponentType<
31-
React.ComponentProps<typeof CamlArticleUntyped> & CamlArticleExtraProps
32-
>;
17+
import { CamlArticle, CamlThemeProvider } from "@os-legal/caml-react";
3318

3419
import { MarkdownMessageRenderer } from "../../threads/MarkdownMessageRenderer";
3520
import {

0 commit comments

Comments
 (0)