Skip to content

Fix clearTimeout using wrong property name in attribute-behavior fixture#684

Closed
everettbu wants to merge 1 commit into
mainfrom
fix/35814-clear-timeout-typo
Closed

Fix clearTimeout using wrong property name in attribute-behavior fixture#684
everettbu wants to merge 1 commit into
mainfrom
fix/35814-clear-timeout-typo

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#36000
Original author: ishaquehassan


The componentWillUnmount method in fixtures/attribute-behavior/src/App.js calls clearTimeout(this.interval), but the timeout ID is stored as this.timeout (set in onMouseEnter). Since this.interval is always undefined, the timer never actually gets cleared when the component unmounts.

Fixes #35814

What changed: one-line fix in fixtures/attribute-behavior/src/App.js, changed clearTimeout(this.interval) to clearTimeout(this.timeout).

componentWillUnmount was calling clearTimeout(this.interval) but the
timeout was stored as this.timeout in onMouseEnter. The timer was never
actually cleared on unmount.

Fixes #35814
@greptile-apps

greptile-apps Bot commented Mar 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a simple but real bug in fixtures/attribute-behavior/src/App.js where componentWillUnmount was calling clearTimeout(this.interval) instead of clearTimeout(this.timeout). Because this.interval is never set anywhere in the component, the timeout ID stored in this.timeout was never cleared on unmount, leaving the pending timer dangling.

  • The fix is a one-line change that aligns componentWillUnmount with onMouseEnter and onMouseLeave, both of which already correctly reference this.timeout.
  • No logic changes, no API surface changes — purely a property name typo correction in a fixture file.

Confidence Score: 5/5

  • This PR is safe to merge — it corrects a clear property name typo with no risk of regression.
  • The change is a single-line typo fix in a fixture file. this.interval is never defined anywhere in the component, so the original code was a no-op. The fix aligns componentWillUnmount with the existing onMouseEnter/onMouseLeave handlers, which already correctly use this.timeout. No behavioral change beyond properly cancelling the pending timeout on unmount.
  • No files require special attention.

Important Files Changed

Filename Overview
fixtures/attribute-behavior/src/App.js One-line bug fix correcting clearTimeout(this.interval) to clearTimeout(this.timeout) in componentWillUnmount, ensuring the timeout is actually cleared on unmount. The fix is consistent with how onMouseEnter and onMouseLeave already reference this.timeout.

Last reviewed commit: 9afb274

@greptile-apps-staging

Copy link
Copy Markdown

Greptile Summary

This PR fixes a minor but real bug in the attribute-behavior fixture where componentWillUnmount called clearTimeout(this.interval) instead of clearTimeout(this.timeout). Because this.interval is never assigned, the guard condition if (this.timeout) would pass but clearTimeout(undefined) would be called — meaning the pending timeout was never actually cancelled on unmount.

  • Root cause: The property name this.interval was used in clearTimeout while the timeout ID is stored as this.timeout (set in onMouseEnter).
  • Fix: Changed clearTimeout(this.interval)clearTimeout(this.timeout), consistent with onMouseLeave and other usages in the same class.
  • Scope: Single line change in a fixture file; no production logic is affected.

Confidence Score: 5/5

  • This PR is safe to merge — it is a straightforward one-line bug fix in a fixture file with no production impact.
  • The change is minimal, clearly correct, and consistent with the existing property usage (this.timeout) in onMouseEnter and onMouseLeave within the same class. There is no risk of regression.
  • No files require special attention.

Important Files Changed

Filename Overview
fixtures/attribute-behavior/src/App.js One-line bug fix: corrects clearTimeout(this.interval) to clearTimeout(this.timeout) in componentWillUnmount, matching the property name used throughout the rest of the class (onMouseEnter, onMouseLeave).

Sequence Diagram

sequenceDiagram
    participant User
    participant Result as Result Component
    participant Timer as setTimeout / clearTimeout

    User->>Result: onMouseEnter
    Result->>Timer: setTimeout(..., 250) → stores as this.timeout
    User->>Result: onMouseLeave (or onMouseEnter again)
    Result->>Timer: clearTimeout(this.timeout) ✅

    Note over Result,Timer: Before fix: componentWillUnmount called clearTimeout(this.interval) ❌
    Note over Result,Timer: After fix: componentWillUnmount calls clearTimeout(this.timeout) ✅

    User->>Result: Component unmounts
    Result->>Timer: clearTimeout(this.timeout) ✅ timer properly cancelled
Loading

Last reviewed commit: 9afb274

@greptile-apps-staging

Copy link
Copy Markdown

🦖 T-Rex

Generated 1 test(s) for 1 file(s): 1 passed, 0 failed, 0 skipped

Test Coverage Summary

Validated lifecycle method correctness in a React class component, specifically ensuring proper cleanup of timers during unmount. The test confirmed that the component correctly references the stored timeout identifier when canceling pending timers, preventing potential memory leaks and setState calls on unmounted components.

Test Results (1 tests)
componentWillUnmount calls clearTimeout with this.timeout, not this.interval MEDIUM fixtures/attribute-behavior/src/App.js (1/1 passed)

Terminal Output

PASS ./App.clearTimeout.test.js
  Result component – timer cleanup on unmount
    ✓ clearTimeout in componentWillUnmount references this.timeout (not this.interval) (2 ms)
    ✓ clearTimeout in componentWillUnmount does NOT reference this.interval (1 ms)
    ✓ onMouseEnter stores the timeout ID into this.timeout (1 ms)
    ✓ this.interval is never assigned in the Result component (1 ms)

Test Suites: 1 passed, 1 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        0.379 s
Ran all test suites matching App.clearTimeout.test.js.

Test Code - fixtures/attribute-behavior/src/App.clearTimeout.test.js

'use strict';

const fs = require('fs');
const path = require('path');

/**
 * Regression test for: clearTimeout uses wrong property in componentWillUnmount.
 *
 * The Result class stores its timeout ID as `this.timeout`.
 * Before the fix, componentWillUnmount called clearTimeout(this.interval)
 * which is always undefined — the timer was never canceled on unmount.
 *
 * This test parses the source of App.js and asserts that within the
 * componentWillUnmount method, clearTimeout is called with `this.timeout`
 * (the correct property) and NOT with `this.interval` (the wrong property).
 */
describe('Result component – timer cleanup on unmount', () => {
  let source;
  let unmountBody;

  beforeAll(() => {
    const appPath = path.resolve(__dirname, 'App.js');
    source = fs.readFileSync(appPath, 'utf-8');

    // Extract the body of componentWillUnmount from the source.
    // The method looks like:
    //   componentWillUnmount() {
    //     if (this.timeout) {
    //       clearTimeout(this.timeout);
    //     }
    //   }
    const match = source.match(
      /componentWillUnmount\s*\(\s*\)\s*\{([\s\S]*?)\n\s*\}/
    );
    expect(match).not.toBeNull();
    unmountBody = match[1];
  });

  test('clearTimeout in componentWillUnmount references this.timeout (not this.interval)', () => {
    // Should contain clearTimeout(this.timeout)
    expect(unmountBody).toMatch(/clearTimeout\s*\(\s*this\.timeout\s*\)/);
  });

  test('clearTimeout in componentWillUnmount does NOT reference this.interval', () => {
    // The bug: clearTimeout(this.interval) — this.interval is always undefined
    expect(unmountBody).not.toMatch(/clearTimeout\s*\(\s*this\.interval\s*\)/);
  });

  test('onMouseEnter stores the timeout ID into this.timeout', () => {
    // Verify the property where the timeout ID is stored matches what unmount clears
    expect(source).toMatch(/this\.timeout\s*=\s*setTimeout\s*\(/);
  });

  test('this.interval is never assigned in the Result component', () => {
    // Extract the Result class body (from its definition to the next top-level class)
    const resultMatch = source.match(
      /class Result extends React\.Component \{([\s\S]*?)class App extends React\.Component/
    );
    expect(resultMatch).not.toBeNull();
    const resultBody = resultMatch[1];

    // this.interval should never be assigned inside Result
    expect(resultBody).not.toMatch(/this\.interval\s*=/);
  });
});

Model: testgen | Tokens: 202,206 (193,808 in, 8,398 out) | Cost: $0.7074

Agent Logs

Generated by Greptile

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Mar 11, 2026
@everettbu
everettbu deleted the fix/35814-clear-timeout-typo branch March 11, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants