Skip to content

fix(runtime): skip generated ids in auto-prefix-id + View.createId bridge (closes #9) - #10

Merged
petermuessig merged 3 commits into
mainfrom
fix/forwarded-aggregation-issue-9
Sep 9, 2026
Merged

petermuessig merged 3 commits into
mainfrom
fix/forwarded-aggregation-issue-9

Conversation

@petermuessig

@petermuessig petermuessig commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Root cause

UI5's View.onControllerConnected uses runWithPreprocessors to apply view.createId as the id-preprocessor for all ManagedObject constructors invoked inside createContent(). Controls like sap.m.Menu create internal helpers (MenuWrapper) whose ids are derived from a parent auto-generated id (e.g. "__menu0-menuWrapper"). With getAutoPrefixId() === true, those derived-generated ids were run through view.createId(), making them unresolvable when the aggregation forwarder later looked them up by id → TypeError: Cannot read properties of … (reading 'addItem').

Fix

Two complementary guards, both using ManagedObjectMetadata.isGeneratedId:

installViewScopeBridge.ts — shadows view.createId with a guarded version for the duration of onControllerConnected. When the runWithPreprocessors id-preprocessor calls view.createId("__menu0-menuWrapper"), the guard returns the id unchanged. The shadow is cleaned up in finally alongside the createContent shadow.

runtime.ts — adds the same isGeneratedId guard to our explicit view.createId() call in the JSX auto-prefix-id block so JSX-authored id props are only prefixed when they are statically supplied by the developer.

Also reverts the incorrect "forwarded aggregation" deferral (defaultAggForwarded + post-construction addAggregation) that was an incorrect symptom-level workaround for the same root cause, and removes the corresponding getAggregation addition from the ControlMetadata SPI type.

Also included (folded in from #11)

  • installViewScopeBridge.ts: wraps createContent() in withScope({ view }) and logs errors via Log.error (sync + async)
  • view-error-logging.qunit.ts: regression tests for the error-logging path
  • ExploreSample.tsx in the showcase: Log.error in both catch blocks

Tests

All 12 suites green (53/53):

Suite Result
runtime/forwarded-aggregation 3/3 ✅
runtime/auto-prefix-id 5/5 ✅
runtime/view-error-logging 2/2 ✅
all others passing

Previously, when a JSX view's createContent() threw (sync or async),
the error propagated as a rejected Promise but was never written to
the UI5 Log. In the showcase it was only rendered as a text control,
invisible to log-scraping tooling and sap-ui-log-level debugging.

The fix patches View.prototype.onControllerConnected (not createContent,
which is bypassed by subclass dynamic dispatch) to install a per-instance
wrapper on this.createContent before calling the original method. The
wrapper opens the JSX scope (withScope), wraps the subclass call in
try/catch plus .catch() for async, calls Log.error on failure, then
re-throws — preserving all existing propagation behaviour.

The instance own-property is deleted in a finally block so the
prototype chain is fully restored after each view construction.

ExploreSample.tsx also adds Log.error calls in both its catch handlers
(demo-view load failure and docs-load failure) so the exact user-facing
message is traceable in the log.

Two QUnit regression tests verify both the sync-throw and async-reject
paths: Log.error is called exactly once with the right message and
component id, and View.create still rejects.

Also brings runtime/forwarded-aggregation.qunit.ts from the
fix/forwarded-aggregation-issue-9 branch (was missing on main).
…nstruction

Controls like sap.m.Menu declare their default aggregation (items) as
forwarded to an internal wrapper control created in init() and resolved
by id at add-time. Passing children via new Type({items}) made
applySettings attempt the lookup before the wrapper was registered,
crashing with a TypeError (issue #9).

The runtime now detects a forwarded default aggregation via
metadata.getAggregation(name)?.forwarding and defers concrete children
to a post-construction addAggregation loop, matching XMLView behaviour.
String-binding and template children are not affected.

- Add getAggregation() to the ControlMetadata structural type (plugin.ts)
- Patch the concrete-children branch in runtime.ts
- Add QUnit regression test (forwarded-aggregation.qunit.ts)
- Add ForwardedMenu showcase example (button menu + List contextMenu)
@petermuessig
petermuessig force-pushed the fix/forwarded-aggregation-issue-9 branch from 3ceea55 to a9c0438 Compare September 9, 2026 15:00
@petermuessig petermuessig changed the title fix(runtime): defer forwarded default-aggregation children to post-construction fix(runtime): defer forwarded default-aggregation children to post-construction + log view-construction errors Sep 9, 2026
…loses #9)

UI5's View.onControllerConnected uses runWithPreprocessors to apply
view.createId as the id-preprocessor for all ManagedObjects constructed
inside createContent(). Controls like sap.m.Menu create internal helpers
(MenuWrapper) whose ids are derived from an auto-generated parent id (e.g.
"__menu0-menuWrapper"). With getAutoPrefixId() === true, these derived-
generated ids were being run through view.createId(), making them
unresolvable when the aggregation forwarder later looked them up by id.

The fix is in two layers:

1. installViewScopeBridge: shadows view.createId with a guard that calls
   ManagedObjectMetadata.isGeneratedId and returns the id unchanged when
   it matches the generated-id pattern (starts with or contains the UID
   prefix, default "__"). This guards the runWithPreprocessors path —
   internal control ids stay stable regardless of the view's autoPrefixId
   setting.

2. runtime.ts: adds the same isGeneratedId guard to the explicit
   view.createId() call in the JSX auto-prefix-id block so JSX-authored
   id props are only prefixed when they are statically supplied by the
   app developer. Also reverts the incorrect "forwarded aggregation"
   deferral (defaultAggForwarded + post-construction addAggregation) that
   was a symptom-level workaround for the same root cause.
@petermuessig petermuessig changed the title fix(runtime): defer forwarded default-aggregation children to post-construction + log view-construction errors fix(runtime): skip generated ids in auto-prefix-id + View.createId bridge (closes #9) Sep 9, 2026
@petermuessig
petermuessig merged commit da9e5db into main Sep 9, 2026
3 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 9, 2026
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.

1 participant