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

fix(utils): make useId SSR friendly #1731

Closed
wants to merge 5 commits into from

Conversation

Al-Rozhkov
Copy link

Describe the PR

If component id is not explicitly specified, it is generated using Math.random(), which leads to hydration errors:

Hydration attribute mismatch on <input id=​"__BVID__921175___BV_form-check__" class=​"form-check-input" type=​"checkbox">​ 
  - rendered on server: id="__BVID__921175___BV_form-check__"
  - expected on client: id="__BVID__935395___BV_form-check__"

Solution: use the bootstrap-vue strategy. If the component id is not specified, leave it empty during SSR and generate it onMounted.

Fixes: #1730

Small replication

A small replication or video walkthrough can help demonstrate the changes made. This is optional, but can help observe the intended changes. A mentioned issue that contains a replication also works.

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

Copy link

stackblitz bot commented Jan 26, 2024

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

Copy link
Member

@VividLemon VividLemon left a comment

Choose a reason for hiding this comment

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

I'll need to think about this. While fixing a different issue for SSR, you've also introduced a https://vuejs.org/guide/scaling-up/ssr.html#cross-request-state-pollution

Regardless, it has other issues such as the debugger and isn't reactive

@Al-Rozhkov
Copy link
Author

Thank you for quick reply. I've fixed debugger.

If no id is specified, it remains empty on the server (just like with bootstrap-vue). getId function is called only client side, so no chance for Cross-Request State Pollution.

Please indicate what other issues you see here.

@darthf1
Copy link
Contributor

darthf1 commented Jan 30, 2024

Nuxt 3.10 just released with an SSR friendly useId composable

For Non-Nuxt users, something similar is expected in Vue 3.5 (but i dont know the timetable).

VividLemon
VividLemon previously approved these changes Jan 31, 2024
@VividLemon
Copy link
Member

VividLemon commented Feb 1, 2024

Just gotta fix the build issues then I'll merge

@Al-Rozhkov
Copy link
Author

I've fixed tests. Please, review again.

VividLemon
VividLemon previously approved these changes Feb 16, 2024
@Al-Rozhkov
Copy link
Author

I double checked build. It should be green now.

@VividLemon
Copy link
Member

Depending on the outcome of Vue 3.5, we may have a builtin composable for this 👀 Perhaps we should simply wait for the official tool first? (With high hopes I suppose, 3.5 may or may not include that change. I'm not mind reader)

It's up to you though. It just looks like a lot of this code is a bit hacky. (nextTicks here and there waiting for things to happen 🤮) I assume the official Vue version would implement SSR protection (as if it didn't, then there would be no actual need for the composable). Idk, just seems like a lot to have a stable id rn

@VividLemon
Copy link
Member

Closing in favor of #1836

@VividLemon VividLemon closed this Apr 3, 2024
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.

Default component ids are not SSR friendly
3 participants