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

KvCheckbox, KvRadio, KvSwitch: Avoid SSR problems when setting the UUID #123

Merged
merged 1 commit into from
Sep 30, 2021

Conversation

ryan-ludwig
Copy link
Contributor

Running into issues on UI with mismatched uuids. This issue does not present itself in storybook. I believe this is caused by SSR setting the UUID on created, then client-side hydration overwriting part of it(?) on mount.
image

Setting in mounted keeps the IDs in sync.

VUE-714

@ryan-ludwig
Copy link
Contributor Author

Curious if anyone has any more insight on this.

@ryan-ludwig
Copy link
Contributor Author

ryan-ludwig commented Sep 30, 2021

Hmm, the tests don't like this, and we'd want to have the UUID there on SSR anyway... Will try some other ways.

@eddieferrer
Copy link
Collaborator

Why don't you set the UUID as a global variable outside the component definition. I don't believe we need two way binding on that anyway.

something like:

const uuid = `kvc-${nanoid(10)}`

export default {
      data() {
		return {
			uuid,
			isChecked: false,
		};
	},
}

@ryan-ludwig
Copy link
Contributor Author

ryan-ludwig commented Sep 30, 2021

@eddieferrer Unfortunately that makes all <kv-radios>s on the page have the same UUID.

@ryan-ludwig
Copy link
Contributor Author

Looks like this is not a new problem.
facebook/react#5867

@ryan-ludwig
Copy link
Contributor Author

The latest push regenerates the ID on mounted keeping the label and input synced.

@ryan-ludwig ryan-ludwig merged commit bc448a9 into main Sep 30, 2021
@ryan-ludwig ryan-ludwig deleted the nanoid-data branch September 30, 2021 22:28
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