fix(runtime-dom): map kebab attributes to camelCase props (fix #13055)

[vonBrax]

Description

This PR fixes an issue where kebab-case attributes are not correctly being set as DOM properties.

The problem is happening because there is no conversion from kebab-case to camelCase in the patchProp function when checking if a property exists on the given element when using the in operator.

The problem also happens when using the .prop modifier, since the patchDOMProp function is called with the given key argument, without checking first for the existence of the camelCase DOM property:

core/packages/runtime-dom/src/patchProp.ts

Line 50 in e16e9a7

patchDOMProp(el, key, nextValue, parentComponent)

The changes in this PR addresses the issue in these two places, by checking first if the camelCase version of the key exists in the Element and using the original key argument as fallback.

Fixes

• close No conversion from kebab-case to camelCase for property binding #13055

Notes

Because of how the current version of patchProp and shouldSetAsProp are structured, it was not possible to avoid making the check with the in operator twice for the cases where shouldSetAsProp was called and executed entirely (or at least I couldn't figure out a way).

I understand this is performance sensitive part of the code so I refactored those functions in a separate branch to show-case a different version where the check is performed at most once per code branch.

I decided to use my first solution for this PR since it was smaller and it didn't introduce style changes, which are discouraged according to the contribution guidelines. But since the same guidelines also suggests attention to performance impacts, I decided to create the second version, which would theoretically be more performant (no benchmarks).

I'm happy to update this PR with the changes from my other branch, if that would so be preferred.

[github-actions]

Size Report

Bundles

File	Size	Gzip	Brotli
runtime-dom.global.prod.js	100 kB (+63 B)	38.2 kB (+17 B)	34.4 kB (+53 B)
vue.global.prod.js	158 kB (+63 B)	58.2 kB (+16 B)	51.9 kB (+48 B)

Usages

Name	Size	Gzip	Brotli
createApp (CAPI only)	46.5 kB (+63 B)	18.2 kB (+12 B)	16.6 kB (+13 B)
createApp	54.4 kB (+63 B)	21.2 kB (+15 B)	19.4 kB (+22 B)
createSSRApp	58.6 kB (+63 B)	22.9 kB (+15 B)	20.9 kB (+7 B)
defineCustomElement	59.2 kB (+63 B)	22.7 kB (+15 B)	20.7 kB (+25 B)
overall	68.5 kB (+63 B)	26.4 kB (+20 B)	24 kB (+37 B)

[pkg-pr-new]

Open in Stackblitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13085

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13085

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13085

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13085

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13085

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13085

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13085

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13085

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13085

vue

npm i https://pkg.pr.new/vue@13085

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13085

commit: 74d74d7

[edison1105]

/ecosystem-ci run

[vue-bot]

📝 Ran ecosystem CI: Open

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

[edison1105]

Maybe we can cache camelKey on el like this:

const camelKey = camelize(key)
if (camelKey in el) {
  ;(el as any)[`$${key}`] = camelKey
  return true
}

patchProp changes as follows

patchDOMProp(el, el[`$${key}`] || key, nextValue, parentComponent)

Call camelize only once, check in only once

[vonBrax]

Yes, we can cache the check result in the element itself. I would just like to point that there are 2 situations where we need to perform the check: for .prop bindings and at the end shouldSetAsProp.
For .prop bindings, check would always be performed and only once.
But if we are coming from shouldSetAsProp, to also avoid performing the in check when we don't need to, I would also need to add the $${key} property on the places where shouldSetAsProp would return true - or alternatively set its default value in the top of the function.

I have a question though: is it possible that el would already have a property with the name we are setting ($${key})? Should we actually add a check for that and what to do if it already exists?

function shouldSetAsProp(...) {
  ...
  const camelKey = camelize(key)
  if (camelKey in el) {
    if ((el as any)[`$${key}`] && (el as any)[`$${key}`] !== camelKey) {
      // What to do? Throw?
    }
     ;(el as any)[`$${key}`] = camelKey
    return true
  }
 ...
}

[vonBrax]

We could use a symbol as key too... either to set directly the value of the current prop being set (would get overwritten by every new attribute being set and not cached across renders) or use it to create an object where properties could then be referenced by their specific key and thus caching the results. Or we leave it as you suggested above. Thoughts?

[edison1105]

Should we actually add a check for that and what to do if it already exists?

We just need to use a key that is as unlikely to duplicate as possible, such as $_v_prop_${key} or symbol as key.