Conversation
Ports SegmentSource from the class-to-hooks migration branch, and
extends the eslint class-component ratchet to this file (scoped to
just SegmentSource.js, since Editarea/SegmentTarget in the same
directory aren't migrated yet).
Destructures the single `segment` prop in the signature. Renames the
render-scope `const {segment} = context` to `contextSegment` to avoid
a collision — the component already tracked these as two distinct
values elsewhere (liveRef.current.segment vs liveRef.current.contextSegment),
this just extends that existing naming to the one spot that didn't use it.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 1 files: 1 ✅ pass
Test File Matching: ✅ PASSFile matching: 1 pass 📋 1 files: 1 ✅ pass
Per-File Evaluation: ✅ PASSAll files resolved by deterministic shortcuts. 📋 1 files: 1 ⏭️ skip
Result: ✅ PASS |
Replaces the 58-method instanceRef/methodsAssignedRef ceremony (needed only so the old Enzyme-style tests could inspect the instance) with plain per-render closures for everything except the handful of functions that genuinely need a stable identity: the three Flux store listeners (registered once, matched by reference) and the two methods tests spy on. Those five share one methodsRef, assigned exactly once, reading current data through a small latestRef mirror instead of the original all-encompassing liveRef. getSearchParams and isValidPhraseToAiAssistant are pure functions of their arguments, so they're now module-level exports, testable directly with no rendering. Rewrites the test file's ~40 ref.current.state.X / direct-method-call assertions to match: real DOM-observable triggers where jsdom allows it, a handful of small explicitly-documented test-only accessors where it doesn't (the same jsdom contentEditable/Selection limitation the original tests already worked around). All 70 tests still pass, same coverage.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 1 files: 1 ✅ pass
Test File Matching: ✅ PASSFile matching: 1 pass 📋 1 files: 1 ✅ pass
Per-File Evaluation: ✅ PASSAll files resolved by deterministic shortcuts. 📋 1 files: 1 ⏭️ skip
Result: ✅ PASS |
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 4 via AI (1 batch), 1 via shortcuts. 📋 5 files: 4 ✅ pass, 1 ⏭️ skip
Result: ✅ PASS |
Removes editAreaClasses/unlockedForCopy state — both were write-only in the class component too, never read in render() or anywhere else, only ever assigned via setState. Converts endSplitMode and refreshTagMap from permanently-stable useRef-wrapped functions to plain per-render closures (via useCallback, depending only on what they actually read), each registered through its own useEffect that re-subscribes to the matching SegmentStore event whenever its dependencies change. setTaggedSource stays permanently stable: its SET_SEGMENT_TAGGED listener is intentionally never removed on cleanup (a pre-existing bug preserved verbatim), so re-registering it on every dependency change would turn that one leaked listener into one leaked per change instead. getSelectedWords similarly becomes a plain closure; the two tests that relied on jest.spyOn(ref.current, 'getSelectedWords') surviving a re-render now mock DraftMatecatUtils.getSelectedTextWithoutEntities instead — more robust, since it no longer depends on the spied instance's identity at all. helpAiAssistant keeps its stable identity: one test asserts its own invocation count across four rapid keyup events, each of which triggers a re-render via its own setIsShowingOptionsToolbar call. latestRef shrinks accordingly (down to what setTaggedSource and helpAiAssistant's delayed setTimeout still read). Net effect: useRef calls 18 → 16, and the two remaining store-listener-driven functions now follow an idiomatic register/cleanup pattern instead of a permanent-registration-plus-live-mirror one.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 4 via AI (1 batch), 1 via shortcuts. 📋 5 files: 4 ✅ pass, 1 ⏭️ skip
Result: ✅ PASS |
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 4 via AI (1 batch), 1 via shortcuts. 📋 5 files: 4 ✅ pass, 1 ⏭️ skip
Result: ✅ PASS |
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSAI analysis failed (Error code: 503 - [{'error': {'code': 503, 'message': 'This model is currently experiencing high demand. Spikes in demand are usually temporary. Please try again later.', 'status': 'UNAVAILABLE'}}]) — shortcuts resolved; remaining files deferred to L1+L2. 📋 5 files: 5 ⏭️ skip
Result: ✅ PASS |
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 4 via AI (1 batch), 1 via shortcuts. 📋 5 files: 4 ✅ pass, 1 ⏭️ skip
Result: ✅ PASS |
…egmentSource's ref
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSAI analysis failed (Error code: 503 - [{'error': {'code': 503, 'message': 'This model is currently experiencing high demand. Spikes in demand are usually temporary. Please try again later.', 'status': 'UNAVAILABLE'}}]) — shortcuts resolved; remaining files deferred to L1+L2. 📋 5 files: 5 ⏭️ skip
Result: ✅ PASS |
… through the DOM SegmentWrapper renders SegmentSource without a ref, so the forwardRef wrapper, the useImperativeHandle surface and its 27 exposed methods existed only to let the tests reach internals. Remove them, along with the stable-assignment guard, the test-only editor accessor and the useReducer that backed a ref forceUpdate. methodsRef stays at three entries (checkDecorators, updateSourceInStore, getSelectedWords), which the call sites frozen relative to render still read to avoid stale closures. Dropping the ref also leaves setSource, setEditorStateBeforeSplit and disableDecorator unreferenced. Tests move to black-box DOM interaction. The TagEntity double now mirrors the real onClickBound, calling back with the entity's own offsets and name, so clicking a tag makes the component force a non-collapsed selection the same way it does in the browser. That reaches the options toolbar, the AI assistant debounce, glossary prefill and split-point removal without a ref. Tests asserting internal state with no user-visible outcome are removed.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSAI analysis failed (Error code: 503 - [{'error': {'code': 503, 'message': 'This model is currently experiencing high demand. Spikes in demand are usually temporary. Please try again later.', 'status': 'UNAVAILABLE'}}]) — shortcuts resolved; remaining files deferred to L1+L2. 📋 5 files: 5 ⏭️ skip
Result: ✅ PASS |
The tag decorator is built once and kept in a ref, so its props are frozen relative to render. 6bf1f4b changed getUpdatedSegmentInfo from a stable function reading the live ref into an arrow capturing contextSegment/tagRange/editorState, freezing them at the first render. TagEntity re-renders itself off EDIT_AREA_CHANGED and calls that prop to pick its warning styling, so a tag that only enters missingTagsInTarget after mount never turned red. Read contextSegment, tagRange, editorState and segment through latestRef, and route onClick through methodsRef, restoring what this.context/this.state gave the class version. onClick was stale the same way, forcing selection against a first-render editorState.
🧪 Test-Guard Report✅ PASS — All changed source files have adequate test coverage. Coverage Analysis: ❌ FAILNo changed source files found in coverage report (threshold: 80%) 📋 5 files: 4 ❌ fail, 1 ✅ pass
Test File Matching: ❌ FAILFile matching: 1 pass, 4 fail 📋 5 files: 4 ❌ fail, 1 ✅ pass
Per-File Evaluation: ✅ PASSEvaluated 5 files: 4 via AI (1 batch), 1 via shortcuts. 📋 5 files: 4 ✅ pass, 1 ⏭️ skip
Result: ✅ PASS |
This was referenced Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Converts
SegmentSourcefrom a class to a function component — the last piece of thesegment editor still on class syntax. Behaviour is unchanged: it keeps its Flux listeners,
the DraftJS decorator pipeline and the split-mode flow.
The test suite is rewritten to drive the component through the DOM instead of an imperative
handle. The conversion had exposed ~27 internal methods via
forwardRef/useImperativeHandlepurely so the class-era tests could keep calling them; no production caller ever passed a ref,
so that surface is gone.
Type
feat— new user-facing featurefix— bug fixrefactor— restructure without behavior changechore— build, deps, config, docsperf— performance improvementtest— test coverageChanges
public/js/components/segments/SegmentSource.jssetStatebecomesuseState, lifecycle becomes effects. Destructures thesegmentprop and renames the context-derived one tocontextSegmentto resolve the collision. Five pure helpers extracted to module scope. NoforwardRef/useImperativeHandle— the tag decorator's props are built once, so they read current data throughlatestRef/methodsRef, which is whatthis.context/this.stategave the class for free.public/js/components/segments/SegmentSource.test.jsTagEntitydouble mirrors the realonClickBound, so clicking a rendered tag exercises the selection-dependent paths (options toolbar, AI assistant, split points) without a ref. Tests asserting internal state with no user-visible outcome removed..eslintrc.jsSegmentSource.jsto the existing override banning class components in migrated files.public/img/icons/*.jsfill-rule/clip-rule→fillRule/clipRule, silencing invalid-DOM-attribute warnings surfaced by the new tests.Testing
vendor/bin/phpunit --exclude-group=ExternalServices --no-coveragepasses./vendor/bin/phpstanpasses (0 errors, with baseline)yarn test— 427 suites / 4384 tests green.SegmentSource.test.jsis 57 tests andSegmentSource.jssits at 88.2% lines, above the 80% floor forpublic/js/components/segments/.Manual testing in the CAT tool caught the regression noted below, which is now covered by
tests. No PHP is touched, so phpunit and phpstan do not apply.
AI Disclosure
Claude Code (Sonnet 5, Opus 5)
Notes
Part of the split of #4768 (wave 2, step 08 of 14) — see #4814 for the full merge-order plan.
Independent of step 09 (Editarea + SegmentTarget); both are only gated on step 07 (TagEntity),
already merged.
One regression was introduced and fixed within this PR: extracting
getUpdatedSegmentInfointo a pure function moved it inside the
useRefinitializer that builds the tag decorator,freezing its inputs at first render, so a tag missing from the target stopped turning red in
the source. Fixed in 71cd271 with regression tests. Net behaviour against
developisunchanged.