fix(runtime): forward JSX key arg to controls that declare a key property - #4
Merged
Merged
Conversation
…erty Babel's automatic JSX transform always extracts the `key` prop from the element's attributes and passes it as the third argument to jsx(type, props, key). It is never present inside the props object. Controls like sap.ui.core.CustomData own a real `key` property (distinct from React's reconciliation key), so the value was silently dropped and the property was never set. The fix: after building propEntries from allProps, push ["key", _key] when _key is defined and the control metadata declares a `key` property. The entry travels through the normal intrinsic + applier pipeline (including propertyTypeIntrinsic validation) exactly like any other prop. For controls without a `key` property the behaviour is unchanged. Fixes #3.
Adds a live showcase sample demonstrating the fix for issue #3: `<CustomData key="product-id" value="4711" writeToDom={true} />` now correctly forwards the JSX `key` attribute to `CustomData.setKey()`, so UI5 writes the expected `data-product-id="4711"` DOM attribute. The sample (Explore → Advanced → "CustomData & the key prop") renders a button with an attached CustomData, reads the DOM attribute back in `onAfterRendering()`, and displays it — giving a visible, in-browser proof that the runtime fix is working end-to-end. Four touchpoints: - webapp/view/showcases/CustomDataKey.tsx (live view) - webapp/docs/samples/custom-data-key.md (description markdown) - webapp/view/ExploreSample.tsx (REGISTRY entry) - webapp/controller/Explorer.controller.ts (EXPLORE_GROUPS leaf)
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3.
Babel's automatic JSX transform always extracts the
keyprop from element attributes and passes it as the third argument tojsx(type, props, key)— it is never inside thepropsobject. Controls likesap.ui.core.CustomDatadeclare a realkeyproperty (unrelated to React's reconciliation concept), so<CustomData key="testkey" .../>compiled to_jsx(CustomData, {...}, "testkey")and the key was silently dropped, producing the empty-key warning from UI5.Changes
runtime.ts: After buildingpropEntries, inject["key", _key]when_keyis defined and the control metadata declares akeyproperty. The entry then travels through the normal intrinsic + applier pipeline (includingpropertyTypeIntrinsicvalidation) exactly like any other prop. For all controls without akeyproperty the behaviour is unchanged.jsx.qunit.ts: Two new tests — one verifyingCustomData.keyis set from the third arg, one confirming controls without akeyproperty don't throw..changeset/fix-key-prop-forwarding.md: Patch-level changeset.CustomDataKey.tsx) that attaches<CustomData key="product-id" value="4711" writeToDom={true} />to a button and reads the resultingdata-product-idattribute back inonAfterRendering()— giving an in-browser proof the fix is working end-to-end. Also removes the motivation for thenew NavigationListItem({ key: "…" })workaround noted inExplorer.controller.ts.Test plan
pnpm --filter @ui5-community/jsx-runtime ts-typecheckpassespnpm --filter @ui5-community/jsx-runtime-showcase ts-typecheckpassespnpm --filter @ui5-community/jsx-runtime test-runner— new tests green, no regressions#/explore/custom-data-key); readout showsDOM attribute: data-product-id="4711"and browser devtools confirm the attribute on the button's root element