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(custom-element): boolean type error in customElement #9698

Conversation

Alfred-Skyblue
Copy link
Member

@Alfred-Skyblue Alfred-Skyblue commented Nov 28, 2023

fixed #9697
fixed #7401

When using defineCustomElement to define a custom element, if we use setAttribute to set attributes for the custom element, the values will be converted to string type. This conversion can lead to type errors when using number or boolean types as props in the component. Currently, we have already handled the conversion for number types in the code. In this PR, I have extended the conversion to boolean types as well. This ensures that boolean type props can be used seamlessly in the CustomElement.

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 86.7 kB (+247 B) 32.9 kB (+54 B) 29.7 kB (+63 B)
vue.global.prod.js 133 kB (+246 B) 49.7 kB (+91 B) 44.6 kB (+48 B)

Usages

Name Size Gzip Brotli
createApp 48 kB 18.9 kB 17.2 kB
createSSRApp 51.2 kB 20.2 kB 18.4 kB
defineCustomElement 50.6 kB (+248 B) 19.7 kB (+58 B) 18 kB (+72 B)
overall 61.4 kB 23.7 kB 21.6 kB

@@ -172,17 +172,19 @@ describe('defineCustomElement', () => {

e.setAttribute('bar', '')
await nextTick()
expect(e.shadowRoot!.innerHTML).toBe(`1 number true boolean 12345 string`)
Copy link
Member Author

Choose a reason for hiding this comment

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

In the original test cases, converting '' to true, I'm not sure if this aligns with expectations, but from a boolean perspective, it should be converted to false.

Choose a reason for hiding this comment

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

HTML Living Spec says that an attribute like bar="" must be interpreted as true. Changing this to false would violate the spec and break exsiting component implementations that use defineCustomElement.

A number of attributes are boolean attributes. The presence of a boolean attribute on an element represents the true value, and the absence of the attribute represents the false value.

If the attribute is present, its value must either be the empty string or a value that is an ASCII case-insensitive match for the attribute's canonical name, with no leading or trailing whitespace.

The values "true" and "false" are not allowed on boolean attributes. To represent a false value, the attribute has to be omitted altogether.

https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a debate over whether, in the absence of a value or when the value is the string 'false,' it should perhaps be considered as false, while in all other cases, it should be considered as true. I have observed that performing such a conversion may result in getAttribute returning different values. It is uncertain whether this approach is reasonable, or if we should adhere to the format of Vue component attributes. However, it is worth noting that we have indeed defined a CustomElement.

@Alfred-Skyblue Alfred-Skyblue changed the title fix: boolean type error in customElement fix(custom-element): boolean type error in customElement Nov 29, 2023
@baiwusanyu-c
Copy link
Member

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Nov 29, 2023

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools failure failure
nuxt success success
pinia success success
quasar success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros success success
vuetify success success
vueuse success success
vue-simple-compiler success success

@Alfred-Skyblue
Copy link
Member Author

#9697 (comment)

@Alfred-Skyblue Alfred-Skyblue deleted the fix/apiCustomElement/props-boolean branch December 7, 2023 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Rejected
5 participants