diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..7a1d539 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,37 @@ +name: CI + +on: + push: + branches: [master] + pull_request: + branches: [master] + +jobs: + test: + runs-on: ubuntu-latest + + strategy: + matrix: + # jsdom 27 requires Node 20.19+ or 22.12+ or 24+ + node-version: [20, 22, 24] + + steps: + - uses: actions/checkout@v4 + + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node-version }} + cache: 'yarn' + + - name: Install dependencies + run: yarn install --frozen-lockfile + + - name: Lint + run: yarn lint + + - name: Test + run: yarn test + + - name: Build + run: yarn build diff --git a/src/ReactDocumentEvents.js b/src/ReactDocumentEvents.js index 5727d33..0be07b9 100644 --- a/src/ReactDocumentEvents.js +++ b/src/ReactDocumentEvents.js @@ -30,10 +30,13 @@ class DocumentEvents extends React.Component { } componentDidUpdate(prevProps) { - if (Object.keys(this.props).sort().toString() !== Object.keys(prevProps).sort().toString()) { - // Handlers passed in likely changed. Rebind. + const keysChanged = Object.keys(this.props).sort().toString() !== Object.keys(prevProps).sort().toString(); + const targetChanged = this.getTarget(prevProps) !== this.getTarget(this.props); + + if (keysChanged || targetChanged) { + // Handlers or target changed. Rebind. this.unbindHandlers(prevProps); - this.bindHandlers(this.props); + if (this.props.enabled) this.bindHandlers(this.props); } else if (!prevProps.enabled && this.props.enabled) { // We became enabled this.bindHandlers(this.props); diff --git a/test/index.spec.js b/test/index.spec.js index c173e65..1794644 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -405,9 +405,7 @@ describe('react-document-events', function () { expect(target.eventListenerCount).to.deep.equal({ click: 0, keydown: 0, mousemove: 0 }); }); - it("should rebind handlers when handler keys change and target differs", () => { - // The component rebinds when the set of prop keys changes. - // When that happens, it unbinds from prevProps (old target) and binds to new props (new target) + it("should rebind handlers when target changes", () => { const target1 = new DummyTarget(); const target2 = new DummyTarget(); const container = document.createElement("div"); @@ -419,12 +417,36 @@ describe('react-document-events', function () { expect(target1.eventListenerCount).to.deep.equal({ click: 1 }); expect(target2.eventListenerCount).to.deep.equal({}); - // Switch targets AND add a new handler (triggers rebind due to key change) + // Switch targets only (same handlers) act(() => { - root.render( {}} onKeyDown={() => {}} />); + root.render( {}} />); }); expect(target1.eventListenerCount).to.deep.equal({ click: 0 }); - expect(target2.eventListenerCount).to.deep.equal({ click: 1, keydown: 1 }); + expect(target2.eventListenerCount).to.deep.equal({ click: 1 }); + + act(() => { + root.unmount(); + }); + expect(target2.eventListenerCount).to.deep.equal({ click: 0 }); + }); + + it("should rebind handlers when target function returns different value", () => { + const target1 = new DummyTarget(); + const target2 = new DummyTarget(); + const container = document.createElement("div"); + const root = createRoot(container); + + act(() => { + root.render( target1} onClick={() => {}} />); + }); + expect(target1.eventListenerCount).to.deep.equal({ click: 1 }); + + // Switch to a function returning a different target + act(() => { + root.render( target2} onClick={() => {}} />); + }); + expect(target1.eventListenerCount).to.deep.equal({ click: 0 }); + expect(target2.eventListenerCount).to.deep.equal({ click: 1 }); act(() => { root.unmount();