Skip to content
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

feat: uniqueId mixin #5567

Closed
wants to merge 4 commits into from

Conversation

i7slegend
Copy link
Contributor

@i7slegend i7slegend commented Apr 11, 2024

Half-duplicate of #5486

#5486 not SSR-friendly (hydration warnings), so i could not find best way for Nuxt.
This MR implements uniqueId mixin (with UniqueComponentId under hood). This mixin uses mounted hook for SSR (like it was before my requests) but for not-SSR calculate unique id before mount that remove redudant re-render (bcz id changes after mount and this trigger re-render for component and his children in the dom tree).

New file: components/lib/utils/UniqueIdMixinFactory.js (placed in primevue/utils). Nuxt is detect by this condition: typeof window === 'undefined' || (typeof process !== 'undefined' && Boolean(process.client || process.server));

Copy link

vercel bot commented Apr 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Visit Preview Apr 11, 2024 0:27am
primevue-v4 ⬜️ Ignored (Inspect) Visit Preview Apr 11, 2024 0:27am

@i7slegend i7slegend marked this pull request as draft April 11, 2024 10:19
@@ -196,10 +196,6 @@ export default {
type: String,
default: null
},
id: {
Copy link
Contributor Author

@i7slegend i7slegend Apr 16, 2024

Choose a reason for hiding this comment

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

@mertsincan mertsincan self-assigned this Apr 19, 2024
@mertsincan mertsincan added the Status: Discussion Issue or pull request needs to be discussed by Core Team label Apr 19, 2024
@mertsincan
Copy link
Member

Good job, @i7slegend! This is exactly what I wanted to do in v4. Currently, I'm working on v4.beta.2 release. I'll review this PR in more detail after beta.2.

@GhostvOne
Copy link

Many hydration issue in console. Thanks @i7slegend. Waiting for the merge :)

@i7slegend
Copy link
Contributor Author

i7slegend commented May 10, 2024

i7slegend@2fc0573

I rewrote from mixin to composable, i bet it's best way to line with vue3 recommendations.
Under SSR i not found any hydration warnings. As Vue library (only client, without ssr) i will test soon time, i think no problems there.

Btw nuxt (since 3.10) have useId built-in composable (https://nuxt.com/docs/api/composables/use-id), maybe should to support it in the SSR environment? But one cons - useId seems doesn't support reactive-based property

@tugcekucukoglu
Copy link
Member

tugcekucukoglu commented Aug 1, 2024

Could you check the latest updates? If you think the problem still exists, please let us know. Closing due to conflicts.

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Discussion Issue or pull request needs to be discussed by Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants