Skip to content

fix: resolve potential mount failure in createTeleport#170

Merged
hexqi merged 3 commits intoopentiny:developfrom
gene9831:fix/create-teleport
Jul 25, 2025
Merged

fix: resolve potential mount failure in createTeleport#170
hexqi merged 3 commits intoopentiny:developfrom
gene9831:fix/create-teleport

Conversation

@gene9831
Copy link
Copy Markdown
Collaborator

@gene9831 gene9831 commented Jul 23, 2025

Summary by CodeRabbit

  • Refactor
    • Improved how teleport targets are determined and passed within popover components, resulting in more direct and dynamic resolution of teleport destinations.
    • Updated internal logic to support both selector strings and HTML elements as teleport targets, enhancing flexibility for component placement.
    • Simplified and modernized component code for better maintainability and performance.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jul 23, 2025

Walkthrough

The changes refactor the teleportation logic in Vue components, updating the way teleport targets are resolved and passed. The approach shifts from using reactive objects to factory functions for teleport props, and updates composables to support both selector strings and HTMLElements as teleport targets. Related imports and function signatures are also adjusted.

Changes

File(s) Change Summary
packages/components/src/base-popper/index.vue
packages/components/src/suggestion-popover/index.vue
Removed unused imports, updated useTeleportTarget calls to accept props.appendTo, refactored teleport prop handling to use factory functions.
packages/components/src/shared/composables/createTeleport.ts Changed createTeleport to accept a factory function for teleport props, deferred rendering to onMounted, updated prop passing.
packages/components/src/shared/composables/useTeleportTarget.ts Updated useTeleportTarget to accept a string or HTMLElement, improved logic for resolving teleport targets, updated JSDoc.

Sequence Diagram(s)

sequenceDiagram
    participant Component as Vue Component
    participant useTeleportTarget as useTeleportTarget
    participant createTeleport as createTeleport
    participant Teleport as Teleport

    Component->>useTeleportTarget: Call with (reference, appendTo)
    useTeleportTarget-->>Component: Returns teleportTarget (HTMLElement or resolved selector)
    Component->>createTeleport: Pass props factory function (returns { to: teleportTarget.value }), child render function
    createTeleport->>Teleport: Mount teleport with dynamic 'to' target
    Teleport-->>Component: Renders children at resolved target
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Poem

A hop, a skip, to teleport anew,
With targets now both string and true,
Factory props, no longer reactive,
Our code’s more clear and interactive.
The portals open, the bunnies cheer—
Refactored magic, far and near!
🐇✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/components/src/base-popper/index.vue

Oops! Something went wrong! :(

ESLint: 9.31.0

Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it.
at /node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:145:10
at async loadTypeScriptConfigFileWithJiti (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:144:3)
at async loadConfigFile (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:266:11)
at async ConfigLoader.calculateConfigArray (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:589:23)
at async #calculateConfigArray (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:743:23)
at async /node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/eslint/eslint.js:767:6
at async Promise.all (index 0)
at async ESLint.lintFiles (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/eslint/eslint.js:764:19)
at async Object.execute (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/cli.js:632:14)
at async main (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/bin/eslint.js:175:19)

packages/components/src/shared/composables/createTeleport.ts

Oops! Something went wrong! :(

ESLint: 9.31.0

Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it.
at /node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:145:10
at async loadTypeScriptConfigFileWithJiti (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:144:3)
at async loadConfigFile (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:266:11)
at async ConfigLoader.calculateConfigArray (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:589:23)
at async #calculateConfigArray (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:743:23)
at async /node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/eslint/eslint.js:767:6
at async Promise.all (index 0)
at async ESLint.lintFiles (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/eslint/eslint.js:764:19)
at async Object.execute (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/cli.js:632:14)
at async main (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/bin/eslint.js:175:19)

packages/components/src/shared/composables/useTeleportTarget.ts

Oops! Something went wrong! :(

ESLint: 9.31.0

Error: The 'jiti' library is required for loading TypeScript configuration files. Make sure to install it.
at /node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:145:10
at async loadTypeScriptConfigFileWithJiti (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:144:3)
at async loadConfigFile (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:266:11)
at async ConfigLoader.calculateConfigArray (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:589:23)
at async #calculateConfigArray (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/config/config-loader.js:743:23)
at async /node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/eslint/eslint.js:767:6
at async Promise.all (index 0)
at async ESLint.lintFiles (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/eslint/eslint.js:764:19)
at async Object.execute (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/lib/cli.js:632:14)
at async main (/node_modules/.pnpm/eslint@9.31.0/node_modules/eslint/bin/eslint.js:175:19)

  • 1 others

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa0ae58 and d23d0f4.

📒 Files selected for processing (4)
  • packages/components/src/base-popper/index.vue (2 hunks)
  • packages/components/src/shared/composables/createTeleport.ts (1 hunks)
  • packages/components/src/shared/composables/useTeleportTarget.ts (2 hunks)
  • packages/components/src/suggestion-popover/index.vue (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: gene9831
PR: opentiny/tiny-robot#123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.
packages/components/src/base-popper/index.vue (3)

Learnt from: gene9831
PR: #59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the show computed property has a custom setter that prevents state mutations when props.trigger === 'manual'. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.

Learnt from: gene9831
PR: #59
File: packages/components/src/suggestion-popover/index.vue:0-0
Timestamp: 2025-05-27T03:45:56.392Z
Learning: In Vue components using VueUse's onClickOutside, calling stopPropagation() in the onClickOutside handler can prevent subsequent click event handlers on the same element from being triggered. This means that clicking a trigger element while a popover is open can successfully close the popover without immediately reopening it, even when the onClickOutside only listens to the popover element and not the trigger element.

Learnt from: gene9831
PR: #123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.

packages/components/src/suggestion-popover/index.vue (3)

Learnt from: gene9831
PR: #59
File: packages/components/src/suggestion-popover/index.vue:131-133
Timestamp: 2025-05-27T03:35:11.008Z
Learning: In the SuggestionPopover component (packages/components/src/suggestion-popover/index.vue), the click handler can be bound unconditionally because the show computed property has a custom setter that prevents state mutations when props.trigger === 'manual'. This design centralizes trigger mode logic in the computed property rather than requiring conditional checks in event handlers.

Learnt from: gene9831
PR: #59
File: packages/components/src/suggestion-popover/index.vue:0-0
Timestamp: 2025-05-27T03:45:56.392Z
Learning: In Vue components using VueUse's onClickOutside, calling stopPropagation() in the onClickOutside handler can prevent subsequent click event handlers on the same element from being triggered. This means that clicking a trigger element while a popover is open can successfully close the popover without immediately reopening it, even when the onClickOutside only listens to the popover element and not the trigger element.

Learnt from: gene9831
PR: #123
File: packages/components/src/bubble/message/Message.vue:22-40
Timestamp: 2025-06-25T07:04:18.791Z
Learning: In the Message component (packages/components/src/bubble/message/Message.vue), the renderer resolution is intentionally not reactive to type changes - the component is designed to work with a fixed type that doesn't change after initialization.

🔇 Additional comments (14)
packages/components/src/suggestion-popover/index.vue (3)

4-4: Good cleanup removing unused import.

The removal of the unused reactive import is appropriate since the reactive teleportProps object was replaced with a factory function.


139-139: Teleport target resolution enhanced with flexibility.

The updated useTeleportTarget call now includes props.appendTo as the second argument, enabling flexible target specification via either HTMLElement or selector string.


141-144: Factory function approach improves teleport initialization.

The refactor from a reactive object to a factory function () => ({ to: teleportTarget.value }) ensures teleport props are evaluated at render time rather than initialization, preventing potential issues with unresolved target elements.

packages/components/src/base-popper/index.vue (3)

4-4: Appropriate cleanup of unused import.

Removing the unused reactive import aligns with the refactor from reactive teleport props to factory functions.


112-112: Enhanced teleport target flexibility.

The useTeleportTarget call now includes props.appendTo, enabling support for both HTMLElement and selector string targets as per the updated composable signature.


116-129: Clean factory function implementation with improved JSX formatting.

The refactor to use a factory function () => ({ to: teleportTarget.value }) ensures proper timing of teleport target resolution. The JSX formatting is clean and readable.

packages/components/src/shared/composables/useTeleportTarget.ts (4)

5-12: Comprehensive JSDoc update reflecting enhanced functionality.

The documentation clearly explains the new dual support for HTMLElement and selector string targets, including the behavior differences for Shadow DOM contexts.


13-17: Efficient early return for HTMLElement targets.

The direct return of HTMLElement instances when provided as the target parameter is an efficient optimization that avoids unnecessary DOM queries.


19-19: Good variable naming to prevent shadowing.

Renaming the parameter to selector after handling the HTMLElement case prevents variable shadowing and improves code clarity.


32-33: Consistent use of renamed variable.

The querySelector call correctly uses the renamed selector variable, maintaining the same functionality while avoiding the shadowing issue.

packages/components/src/shared/composables/createTeleport.ts (4)

1-11: Appropriate import additions for lifecycle management.

The addition of onMounted and other Vue imports supports the improved timing control for teleport creation.


13-20: Factory function pattern correctly implemented in component.

The TeleportWrapperComponent now properly calls the factory functions at render time, enabling dynamic evaluation of teleport props and content.


22-22: Enhanced function signature supporting factory pattern.

The updated signature createTeleport(props: () => TeleportProps, child: () => VNode) correctly defines the factory function expectations.


27-35: Proper timing control with onMounted lifecycle.

Moving teleport creation inside onMounted ensures the component is fully initialized before teleport rendering occurs, preventing potential issues with unresolved target elements. The existing nextTick wrapper provides additional timing safety.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@gene9831 gene9831 changed the title Fix/create teleport refactor: enhance teleport target and Popper component Flexibility Jul 25, 2025
@gene9831 gene9831 marked this pull request as ready for review July 25, 2025 01:38
@gene9831 gene9831 changed the title refactor: enhance teleport target and Popper component Flexibility fix: resolve potential mount failure in createTeleport Jul 25, 2025
@hexqi hexqi merged commit ea332b2 into opentiny:develop Jul 25, 2025
@gene9831 gene9831 deleted the fix/create-teleport branch July 26, 2025 16:39
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.

3 participants