From 2aa625df8d9084f0efb262afa4763c08f13b7259 Mon Sep 17 00:00:00 2001 From: Ryuuji Yoshimoto Date: Sun, 9 Aug 2026 22:34:45 +0900 Subject: [PATCH] =?UTF-8?q?fix(view):=20=E6=96=87=E5=AD=97=E5=88=97ref?= =?UTF-8?q?=E3=81=A8findDOMNode=E3=82=92createRef=E3=81=AB=E7=BD=AE?= =?UTF-8?q?=E3=81=8D=E6=8F=9B=E3=81=88=E3=80=81unmount=E6=99=82=E3=81=AB?= =?UTF-8?q?=E3=83=AA=E3=82=B9=E3=83=8A=E3=83=BC=E3=82=92=E8=A7=A3=E9=99=A4?= =?UTF-8?q?=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Indexが componentDidMount で登録する popstate リスナーに解除処理が無く、 scroll / resize も bind した別参照で登録して生メソッドで removeEventListener していたため解除できていなかった。littel-ui のようにマウントし直す使い方では unmount 済みインスタンスのリスナーが残り、popstate のたびに剥がされた this.refs.results (undefined) へ setState して 「Cannot read properties of undefined (reading 'setState')」が出ていた。 - 文字列 ref (box / freeword / results) を createRef に置き換える。 freeword と box は元からホスト要素なので findDOMNode は呼び出しごと外せる - リスナーを束縛済み参照でフィールドに持ち、componentWillUnmount で popstate / scroll / resize をすべて解除する。resizeTimer も clearTimeout する - window.pressKey も自分が設定したものなら unmount 時に片付ける - onPopState が setState に渡していた sort_key はタイポ (正しくは sort_column)。 popstate で戻った時にソート列がリセットされていなかった - onSort / onSelectBook の引数型を React.SyntheticEvent に直す。 onChange 用の型 (ChangeEvent) のまま onClick / onKeyUp に渡されていて、 strictBindCallApply を有効にすると型エラーになる唯一の箇所だった - Index をクライアント描画するテストを追加。unmount 後にリスナーが 反応しないことを検証する (修正前のコードでは5件すべて落ちる) createRef は React 16.3 以降の API なので React 18 据え置きのまま入れられる。 これで littel-ui / unitrad-kintone-plugin / unitrad-ui-nagano が React 19 に 上げられない原因 (findDOMNode と文字列 ref) が上流から消える。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ERVipW4efqpQTjkKaqVoXr --- src/js/view/index.tsx | 49 ++++++++++++++--------- src/js/view/result.tsx | 10 ++--- test/test_view.mts | 89 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 24 deletions(-) diff --git a/src/js/view/index.tsx b/src/js/view/index.tsx index d4e9e6d..c490227 100644 --- a/src/js/view/index.tsx +++ b/src/js/view/index.tsx @@ -9,7 +9,6 @@ */ import React from 'react'; -import {findDOMNode} from 'react-dom'; import Results from './result' import {DefaultHoldingView} from './holding' import {normalizeQuery, isEmptyQuery, fetchMapping} from '../api' @@ -100,6 +99,13 @@ export default class Index extends React.Component { requestUpdateURL: null | 'search' | 'filter'; resizeTimer: number | null | undefined; + boxRef = React.createRef(); + freewordRef = React.createRef(); + resultsRef = React.createRef(); + /* removeEventListenerに同じ参照を渡すため、リスナーはここで束縛して持つ */ + boundOnPopState = (e: PopStateEvent) => this.onPopState(e); + boundOnScroll = () => this.onScroll(); + boundOnPressKey = (word: string) => this.onPressKey(word); constructor(props: Props) { super(props); @@ -135,12 +141,12 @@ export default class Index extends React.Component { } componentDidMount() { - window.pressKey = this.onPressKey.bind(this); + window.pressKey = this.boundOnPressKey; if (typeof history !== 'undefined' && history.pushState && history.state !== undefined) { - window.addEventListener('popstate', (e) => this.onPopState(e)); + window.addEventListener('popstate', this.boundOnPopState); } - window.addEventListener("scroll", this.onScroll.bind(this)); - window.addEventListener("resize", this.onScroll.bind(this)); + window.addEventListener("scroll", this.boundOnScroll); + window.addEventListener("resize", this.boundOnScroll); if (!(this.props.region in this.state.mapping) || Object.keys(this.state.mapping[this.props.region].libraries).length === 0) { fetchMapping(this.props.region, (res) => { this.state.mapping[this.props.region] = res; @@ -150,15 +156,18 @@ export default class Index extends React.Component { } componentWillUnmount() { - window.removeEventListener("scroll", this.onScroll); - window.removeEventListener("resize", this.onScroll); + window.removeEventListener('popstate', this.boundOnPopState); + window.removeEventListener("scroll", this.boundOnScroll); + window.removeEventListener("resize", this.boundOnScroll); + if (this.resizeTimer) clearTimeout(this.resizeTimer); + if (window.pressKey === this.boundOnPressKey) delete window.pressKey; } onScroll(e?: Event | React.SyntheticEvent) { if (this.resizeTimer) clearTimeout(this.resizeTimer); this.resizeTimer = window.setTimeout(() => { - let element = findDOMNode(this.refs.box); - if (element && element instanceof HTMLElement) { + let element = this.boxRef.current; + if (element) { let rect = element.getBoundingClientRect(); let windowHeight: number = (window.innerHeight || 0); this.setState({logoAvailable: windowHeight - 50 > rect.top + rect.height}) @@ -177,7 +186,8 @@ export default class Index extends React.Component { query: normalizeQuery(params), established_query: normalizeQuery(params) }); - (this.refs.results as any).setState({selected_id: getHash(), page: 0, sort_key: null, sort_order: ''}); + /* sort_column: 従来はタイポでsort_keyを渡していて、ソート列がリセットされていなかった */ + this.resultsRef.current?.setState({selected_id: getHash(), page: 0, sort_column: '', sort_order: ''}); } doSearch(e: React.SyntheticEvent) { @@ -197,7 +207,7 @@ export default class Index extends React.Component { isbn: this.state.query.isbn ? this.state.query.isbn : '' }; } - (this.refs.results as any).setState({selected_id: null, page: 0, sort_column: null, sort_order: ''}); + this.resultsRef.current?.setState({selected_id: null, page: 0, sort_column: '', sort_order: ''}); this.setState({established_query: normalizeQuery(query)}); let onSearch = this.props.onSearch || null; if (onSearch) onSearch(normalizeQuery(query)); @@ -212,7 +222,7 @@ export default class Index extends React.Component { } else if (word === '[search]') { let query: UnitradQuery; query = {free: this.state.query.free ? this.state.query.free : ''}; - (this.refs.results as any).setState({selected_id: null, page: 0, sort_column: null, sort_order: ''}); + this.resultsRef.current?.setState({selected_id: null, page: 0, sort_column: '', sort_order: ''}); this.setState({established_query: normalizeQuery(query)}); let onSearch = this.props.onSearch || null; if (onSearch) onSearch(normalizeQuery(query)); @@ -225,7 +235,8 @@ export default class Index extends React.Component { freeword = window.jaco!.remove(freeword, /゜|゚|ﾟ/g); this.state.query.free = freeword; this.setState({}); - const elm = findDOMNode(this.refs.freeword) as any; + const elm = this.freewordRef.current as any; + if (!elm) return; elm.focus(); if (elm.createTextRange) { var range = elm.createTextRange(); @@ -283,7 +294,7 @@ export default class Index extends React.Component { } else { this.setState(newState as any); } - (this.refs.results as any).setState({page: 0}); + this.resultsRef.current?.setState({page: 0}); } changeCustom(e: React.ChangeEvent) { @@ -315,7 +326,7 @@ export default class Index extends React.Component { if (history.pushState && history.state !== undefined) { let query_string = buildQueryString(this.state.established_query, this.state.mode, this.state.filter); if ('?' + location.search.split('?')[1] !== query_string) { - let hash = ((this.refs.results as any).state.selected_id && this.requestUpdateURL === 'filter') ? '#' + (this.refs.results as any).state.selected_id : ''; + let hash = (this.resultsRef.current?.state.selected_id && this.requestUpdateURL === 'filter') ? '#' + this.resultsRef.current.state.selected_id : ''; history.pushState('search', '', location.pathname + query_string + hash); } } @@ -325,12 +336,12 @@ export default class Index extends React.Component { let form; if (this.state.mode === 'simple') { form = ( -
+
@@ -350,7 +361,7 @@ export default class Index extends React.Component { } }; form = ( -
+
@@ -426,7 +437,7 @@ export default class Index extends React.Component { ); } })()} - { if (this.api) this.api.kill(); } - onSelectBook(e: React.ChangeEvent) { + onSelectBook(e: React.SyntheticEvent) { if (window.getSelection().toString() !== '') return; // 選択中はクリックを処理しない - let current: Element | null | undefined = e.target; + let current: Element | null | undefined = e.target as Element; while (current && current.parentNode) { if (current.attributes.getNamedItem('data-id')) { let hash = current.attributes.getNamedItem('data-id').value; @@ -161,9 +161,9 @@ export default class Results extends React.Component { this.setState({page: data.selected, selected_id: null}); } - onSort(e: React.ChangeEvent) { + onSort(e: React.SyntheticEvent) { this.removeHash(); - let target: null | Element & HTMLElement = e.target; + let target: null | Element & HTMLElement = e.target as Element & HTMLElement; while (target && !target.className.match('sort')) { target = target.parentElement; } @@ -198,7 +198,7 @@ export default class Results extends React.Component { e = e || (window.event as any); if (e.keyCode === 13) { e.stopPropagation(); - this.onSort(e as any); + this.onSort(e); } } diff --git a/test/test_view.mts b/test/test_view.mts index cb5e051..f35ad7f 100644 --- a/test/test_view.mts +++ b/test/test_view.mts @@ -17,6 +17,8 @@ const g = globalThis as any; g.window = dom.window; g.document = dom.window.document; g.location = dom.window.location; +/* Index の componentDidMount が popstate リスナーを登録する条件に history を見る */ +g.history = dom.window.history; /* navigator はNode 21以降 読み取り専用なので触らない。renderToStringでは使われない */ g.HTMLElement = dom.window.HTMLElement; g.Element = dom.window.Element; @@ -167,6 +169,93 @@ describe('Index（検索ボックス）', () => { }); }); +describe('Index（クライアント描画とライフサイクル）', () => { + /* libraries を渡しておくと componentDidMount の fetchMapping が通信を起こさない */ + const base = { + region: 'test', mode: 'simple', + filters: [{id: 0, name: '全域', includes: []}], + libraries: {1: 'A図書館'}, name_to_id: {'A図書館': [1]} + }; + + before(() => setSearch('')); + + function mountIndex() { + const container = dom.window.document.createElement('div'); + dom.window.document.body.appendChild(container); + const root = createRoot(container); + let instance: any = null; + const ref = (r: any) => { if (r) instance = r; }; + act(() => { + root.render(React.createElement(Index, {...base, ref} as any)); + }); + return { + instance, + unmount() { + act(() => { root.unmount(); }); + container.remove(); + } + }; + } + + it('resultsRefから結果一覧のインスタンスに触れる', () => { + const m = mountIndex(); + assert.ok(m.instance.resultsRef.current); + m.unmount(); + }); + + it('popstateで結果一覧の選択とソートをリセットする', () => { + const m = mountIndex(); + const results = m.instance.resultsRef.current; + act(() => { + results.setState({selected_id: 'b1', page: 3, sort_column: 'title', sort_order: 'ascend'}); + }); + act(() => { + dom.window.dispatchEvent(new dom.window.PopStateEvent('popstate')); + }); + assert.equal(results.state.selected_id, ''); + assert.equal(results.state.page, 0); + /* sort_column は従来 sort_key へのタイポでリセットされていなかった */ + assert.equal(results.state.sort_column, ''); + assert.equal(results.state.sort_order, ''); + m.unmount(); + }); + + /* + リスナーを解除せずに unmount すると、後続の popstate が unmount 済みインスタンスの + onPopState を叩き、剥がされた ref (undefined) への setState で TypeError になっていた。 + littel-ui のようにマウントし直す使い方で「Cannot read properties of undefined + (reading 'setState')」が出ていた原因。 + */ + it('unmountするとpopstateに反応しなくなる', () => { + const m = mountIndex(); + let called = 0; + m.instance.onPopState = () => { called++; }; + dom.window.dispatchEvent(new dom.window.PopStateEvent('popstate')); + assert.equal(called, 1); + m.unmount(); + dom.window.dispatchEvent(new dom.window.PopStateEvent('popstate')); + assert.equal(called, 1); + }); + + it('unmountするとscroll/resizeに反応しなくなる', () => { + const m = mountIndex(); + let called = 0; + m.instance.onScroll = () => { called++; }; + dom.window.dispatchEvent(new dom.window.Event('resize')); + assert.equal(called, 1); + m.unmount(); + dom.window.dispatchEvent(new dom.window.Event('resize')); + assert.equal(called, 1); + }); + + it('unmountでwindow.pressKeyを片付ける', () => { + const m = mountIndex(); + assert.equal(typeof dom.window.pressKey, 'function'); + m.unmount(); + assert.equal(dom.window.pressKey, undefined); + }); +}); + describe('Results（検索結果）', () => { const base = { filter: 0, filters: [{id: 0, name: '全域', includes: []}], excludes: [], selected_id: null,