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(shared): normalize undefined styles to empty object #4424

Closed
wants to merge 1 commit into from

Conversation

nVitius
Copy link

@nVitius nVitius commented Aug 23, 2021

Currently, setting v-bind:style to a value that can compute to undefined will remove any styles on the element when it does return undefined. I ran into this issue when using v-show along with an undefined style binding.

This patch makes it so that the normalizeStyle function will convert undefined/null bindings to an empty object instead. I feel like this is a good place to add the check, but it is my first patch on this repo. If anyone can think of a better place to do this, I will change it.

packages/shared/src/normalizeProp.ts Outdated Show resolved Hide resolved
packages/shared/src/normalizeProp.ts Outdated Show resolved Hide resolved
@nVitius
Copy link
Author

nVitius commented Aug 25, 2021

@yyx990803 Thanks for the review. I made the requested changes.

@Justineo
Copy link
Member

Thank you for the PR. Can you make a minimal reproduction for the issue you are trying to fix?

I tried to make a repro but it does work as expected (v-show + :style="undefined").

And according to the coercion behavior for Vue 3, undefined bindings should lead to the removal of the attribute. Always normalizing undefined to {} will change this behavior.

@nVitius
Copy link
Author

nVitius commented Aug 31, 2021

@Justineo I spent some time in the SFC playground today, trying to get a reproduction working (not working?).
I could not replicate the issue on there. From what I could tell, the compiled JS I got in the playground was the same as the JS I saw in the library that had the issue. Not really sure what to make of this.

I don't really have any more time to spend on setting trying to reproduce the issue. We fixed it in the library by using an empty object instead of undefined. I'm okay with closing this issue. Thanks for your help.

And according to the coercion behavior for Vue 3, undefined bindings should lead to the removal of the attribute. Always normalizing undefined to {} will change this behavior.

This is really interesting to know, thanks. For what it's worth, I was getting an empty style, as opposed to the attribute being removed: <div style="">.

@yyx990803
Copy link
Member

I was able to reproduce this: link

(Clicking "remove" should not cause the input to be shown)

@yyx990803 yyx990803 closed this in d534515 Sep 7, 2021
@yyx990803
Copy link
Member

After looking into it I believe the more proper fix is ensuring style patching always respect v-show's inline display value (there was already logic in there but not covering the removal case)

@nVitius
Copy link
Author

nVitius commented Sep 9, 2021

@yyx990803 Thanks for the help with diagnosing this. When you create the patch for that, will you link to it here? I'm interested in seeing where that happens. Thanks!

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.

None yet

3 participants