Skip to content

fix(dialogs/spawnDialog)!: support vue-devtool but lose appContext #6752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 8, 2025

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Apr 7, 2025

☑️ Resolves

This looses appContext.

Workarounds:

  • Always use locally imported components
  • When possible - provide data via props
  • For provide/inject - provide directly from the root dialog component
  • For Pinia - should work via activePinia, or via provide in the root dialog component

Alternative:

  • Pass app/appContext via options and then do hacks - re-provide every provider

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport bugfixes to stable8 for maintained Vue 2 version.

@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews 💥 breaking PR that requires a new major version feature: functions composables, functions, mixins and other non-components labels Apr 7, 2025
@ShGKme ShGKme added this to the v9.0.0-rc.0 milestone Apr 7, 2025
@ShGKme ShGKme requested a review from susnux April 7, 2025 21:09
@ShGKme ShGKme self-assigned this Apr 7, 2025
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

makes sense and is cleaner, but quite some negative aspects for just some DX.

@susnux susnux changed the title fix(dialogs/spawnDialog): support vue-devtool but loose appContext fix!(dialogs/spawnDialog): support vue-devtool but loose appContext Apr 7, 2025
@ShGKme ShGKme changed the title fix!(dialogs/spawnDialog): support vue-devtool but loose appContext fix(dialogs/spawnDialog)!: support vue-devtool but loose appContext Apr 7, 2025
@ShGKme
Copy link
Contributor Author

ShGKme commented Apr 7, 2025

makes sense and is cleaner, but quite some negative aspects for just some DX.

I tried many options and found no way to have both app context and independent rendering.

We can either hack with re-registering all global component, re-providing everything from providers.

Or manually add vnode to the parent's children on beforeUpdate + update (IF there is a parent, and it's not mounted outside Vue apps).

@ShGKme
Copy link
Contributor Author

ShGKme commented Apr 7, 2025

And as far as I know, there is no case where we need it at the moment.

@susnux
Copy link
Contributor

susnux commented Apr 7, 2025

We can either hack with re-registering all global component, re-providing everything from providers.

I think this is quite complex solution for a rare condition (when there is even one). So yes lets stick with this PR.

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

LGTM. Let's see how it works in app, when we get to it

@ShGKme ShGKme merged commit db5d559 into main Apr 8, 2025
24 checks passed
@ShGKme ShGKme deleted the fix/spawnDialog-createApp branch April 8, 2025 08:13
@ShGKme ShGKme changed the title fix(dialogs/spawnDialog)!: support vue-devtool but loose appContext fix(dialogs/spawnDialog)!: support vue-devtool but lose appContext Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews 💥 breaking PR that requires a new major version bug Something isn't working feature: functions composables, functions, mixins and other non-components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants