🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@IGx89
Copy link
Contributor

@IGx89 IGx89 commented Sep 22, 2025

Describe the PR

The BToggle directive searches for targets in its mounted hook. If a target isn't immediately found, it calls setTimeout and keeps looking for it for 400ms. If the directive is unmounted before that 400ms is up, it still keeps looking even though there's no point anymore. This PR adds a check on that loop that exits it if the directive has been unmounted.

Small replication

    const App = {
      directives: {
        bToggle: VBToggle,
      },
      components: {BCollapse},
      template: `<button v-if="show" v-b-toggle="'missing'">button</button>`,
      setup: () => {
        const show = ref(true)
        onMounted(() => {
          show.value = false
        })
        return {
          show,
        }
      },
    }

    // When mounted will produce a console warn with text "[v-b-toggle] Target with ID missing not found". After this change it won't.

PR checklist

What kind of change does this PR introduce? (check at least one)

  • Bugfix 🐛 - fix(...)
  • Feature - feat(...)
  • ARIA accessibility - fix(...)
  • Documentation update - docs(...)
  • Other (please describe)

The PR fulfills these requirements:

  • Pull request title and all commits follow the Conventional Commits convention or has an override in this pull request body This is very important, as the CHANGELOG is generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be denied

Summary by CodeRabbit

  • Bug Fixes
    • Improved stability of the toggle directive when elements are unmounted, preventing redundant processing after detachment.
    • Reduces rare edge-case flicker and potential console warnings/errors when targets are removed.
    • No behavior or API changes required for users; existing setups continue to work as before.

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

Walkthrough

The vBToggle directive’s internal loop now includes a guard to stop iterating if the element’s dataset.bvtoggle is cleared (e.g., after unmount). Only the while-condition changed; comments were added. No public APIs or other logic paths were modified.

Changes

Cohort / File(s) Summary
Directive loop guard update
packages/bootstrap-vue-next/src/directives/BToggle/index.ts
Updated while-condition to count < 5 && el.dataset.bvtoggle to exit early when unmounted; added explanatory comment. No API or behavior changes beyond the guard.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor C as Component
  participant D as vBToggle Directive
  participant E as Element (dataset.bvtoggle)
  participant T as Target Resolver

  C->>D: mount/update
  D->>E: read dataset.bvtoggle
  loop up to 5 iterations with guard (stops if bvtoggle cleared)
    alt bvtoggle present
      D->>T: resolve show/hide targets
      T-->>D: target(s) found or not
    else bvtoggle cleared (unmounted)
      Note over D,E: Guard exits loop immediately
    end
  end
  C-->>D: unmount (clears dataset.bvtoggle)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • VividLemon

Poem

I twitch my ears at tidy loops,
A gentle hop, then safely stoops—
If toggle’s gone, I won’t persist,
I nose the air, I don’t insist.
Five hops max, then I desist—
A careful bun, with guarded twist. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the primary change — preventing BToggle from continuing to search for missing targets after the directive is unmounted. It names the affected symbol (BToggle) and the behavior being fixed, making it clear for reviewers and changelog generation. The title follows Conventional Commits formatting with the fix(...) scope.
Description Check ✅ Passed The PR description follows the repository template: it includes a clear "Describe the PR" section explaining the bug and the fix, a "Small replication" with runnable code demonstrating the issue, and the PR checklist with the bugfix and Conventional Commits boxes checked. The reproduction example and expected behavior are explicit and sufficient for reviewers to validate the change. Therefore the description is mostly complete and actionable.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 22, 2025

bsvn-vite-ts

npm i https://pkg.pr.new/bootstrap-vue-next/bootstrap-vue-next@2857
npm i https://pkg.pr.new/bootstrap-vue-next/bootstrap-vue-next/@bootstrap-vue-next/nuxt@2857

commit: f1076c8

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/bootstrap-vue-next/src/directives/BToggle/index.ts (1)

69-75: Re-check unmounted state after the await to avoid a late warn.

If the directive unmounts during the 100ms delay on the final attempt, a warning can still fire. Add a post-await guard before logging.

Apply this diff:

-        count++
-        await new Promise((resolve) => setTimeout(resolve, 100))
+        count++
+        await new Promise((resolve) => setTimeout(resolve, 100))
+        // Abort if directive got unmounted during the delay
+        if (!(el as HTMLElement).dataset.bvtoggle) break
         if (count < 4) continue
         // eslint-disable-next-line no-console
         console.warn(`[v-b-toggle] Target with ID ${targetId} not found`)
         break
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0963e44 and f1076c8.

📒 Files selected for processing (1)
  • packages/bootstrap-vue-next/src/directives/BToggle/index.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (1)
packages/bootstrap-vue-next/src/directives/BToggle/index.ts (1)

65-66: Good guard to stop retries after unmount.

The added condition prevents post-unmount polling and aligns with the PR goal.

Copy link
Contributor

@xvaara xvaara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice little check!

@xvaara xvaara merged commit b358449 into bootstrap-vue-next:main Sep 23, 2025
5 checks passed
@github-actions github-actions bot mentioned this pull request Sep 23, 2025
xvaara added a commit to xvaara/bootstrap-vue-next that referenced this pull request Oct 11, 2025
* upstream/main:
  chore: release main (bootstrap-vue-next#2868)
  fix(useModalOrchestrator): circular dependency (bootstrap-vue-next#2874)
  docs(BModal): Parity pass (bootstrap-vue-next#2866)
  docs: Enable directly loading examples into StackBlitz (bootstrap-vue-next#2869)
  fix(BApp): wrap our test app in BApp in main.ts to enable easy verification of useModal, etc. (bootstrap-vue-next#2865)
  export useScrollLock() (bootstrap-vue-next#2854)
  chore: release main (bootstrap-vue-next#2858)
  fix(BToggle): stop looking for missing targets after directive is unmounted (bootstrap-vue-next#2857)
  chore: release main (bootstrap-vue-next#2851)
  Fix modal transition delays by making TransitionGroup name conditional (bootstrap-vue-next#2845)
  chore: release main (bootstrap-vue-next#2842)
  fix(BTable): events being wrongly stopped when sent from elements inside TRs (bootstrap-vue-next#2841)
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