tiny-engine icon indicating copy to clipboard operation
tiny-engine copied to clipboard

Fix/element not found v2.7:Add condition for when element is null

Open betterdancing opened this issue 2 months ago • 1 comments

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] The commit message follows our Commit Message Guidelines
  • [ ] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)
  • [ ] Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • [x] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

Background and solution

What is the current behavior?

elementId存在但是找不到对应的element元素,例如teleport 或者是 顶层是 fragment 的元素 Issue Number: N/A

What is the new behavior?

如果在elementId找不到对应的元素,则直接return默认的delay毫秒50ms

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of missing elements in duration calculations with optimized default timing behavior.

betterdancing avatar Oct 31 '25 02:10 betterdancing

[!WARNING]

Rate limit exceeded

@betterdancing has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe8623ed82e887863cfcb7f6ffe51548c1fe4ed and c335dd415791d659e7342a25ba37d34e18b2f609.

📒 Files selected for processing (1)
  • packages/canvas/container/src/container.ts (2 hunks)

Walkthrough

The getElementDurationTime function in the canvas container was modified to return early with a 50ms default delay when the target element is not found, instead of proceeding with duration computation. Valid elements continue using the existing logic.

Changes

Cohort / File(s) Summary
Canvas Container Duration Logic
packages/canvas/container/src/container.ts
Modified getElementDurationTime to default delayTime to 50ms and return immediately when target element is missing, rather than continuing to compute durations

Sequence Diagram

sequenceDiagram
    participant Caller
    participant getElementDurationTime
    participant DOM

    rect rgb(240, 248, 255)
    Note over Caller,DOM: Old Behavior (no early return)
    Caller->>getElementDurationTime: query element
    getElementDurationTime->>DOM: find element
    DOM-->>getElementDurationTime: not found (null)
    getElementDurationTime->>getElementDurationTime: proceed with duration computation
    getElementDurationTime-->>Caller: computed value
    end

    rect rgb(230, 245, 220)
    Note over Caller,DOM: New Behavior (with early return)
    Caller->>getElementDurationTime: query element
    getElementDurationTime->>DOM: find element
    DOM-->>getElementDurationTime: not found (null)
    getElementDurationTime-->>Caller: return 50ms (default)
    end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify existing callers handle the early return correctly
  • Confirm the 50ms default is appropriate for all use cases
  • Check if test coverage exists for the missing element scenario

Poem

🐰 A swift return when paths don't align, Fifty milliseconds, a graceful design, No more wandering through absent nodes, Early exits smooth the coded roads! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Fix/element not found: Add condition for when element is null" directly addresses the core change described in the file summary and PR objectives. The title explicitly mentions "element not found" and "condition for when element is null," which accurately reflects the main change: adding a condition to handle cases where an elementId exists but the corresponding DOM element cannot be resolved, causing the function to return the default delay value of 50ms. The title is concise, specific, and clearly conveys the primary change to a teammate reviewing the history.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Oct 31 '25 02:10 coderabbitai[bot]