feat(dts-generator): trust source data for event parameter optionality#576
Open
akudev wants to merge 1 commit into
Open
feat(dts-generator): trust source data for event parameter optionality#576akudev wants to merge 1 commit into
akudev wants to merge 1 commit into
Conversation
akudev
commented
May 20, 2026
akudev
commented
May 20, 2026
There was a problem hiding this comment.
Pull request overview
Updates the UI5 .d.ts generator to stop forcing all event-parameter properties to be optional, and instead honor the optional flag from api.json, with an escape hatch for events whose optionality is not yet trusted.
Changes:
- Adjust event-parameter interface generation to make properties required when
optional: false(defaulting to optional whenoptionalis missing/true). - Introduce a new
.dtsgenrcdirectiveeventsWithAllParamsOptionalto preserve legacy “all params optional” behavior for specified events. - Add a dedicated test suite and regenerate OpenUI5 snapshot
.d.tsoutputs to reflect the new optionality rules.
Reviewed changes
Copilot reviewed 6 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test-packages/openui5-snapshot-test/test/event-params-optionality-spec.js | Adds targeted mocha tests for event parameter optionality, deny-list behavior, and inheritance. |
| packages/dts-generator/src/utils/json-event-parameter-interfaces.ts | Implements optionality logic + deny-list support when building $Xxx$YyyEventParameters interfaces. |
| packages/dts-generator/src/phases/json-fixer.ts | Passes directives into event-parameter interface generation and enables overlays to target events by name. |
| packages/dts-generator/src/generate.ts | Initializes the new directive field when loading/merging directives. |
| packages/dts-generator/src/generate-from-objects.ts | Extends the public Directives type with eventsWithAllParamsOptional. |
| packages/dts-generator/api-report/dts-generator.api.md | Updates the API report to include the new directive field. |
| test-packages/openui5-snapshot-test/output-dts/sap.ui.unified.d.ts | Regenerated snapshot reflecting required event parameters where optional: false. |
| test-packages/openui5-snapshot-test/output-dts/sap.ui.layout.d.ts | Regenerated snapshot reflecting required event parameters where optional: false. |
| test-packages/openui5-snapshot-test/output-dts/sap.ui.core.d.ts | Regenerated snapshot reflecting required event parameters where optional: false. |
| test-packages/openui5-snapshot-test/output-dts/sap.tnt.d.ts | Regenerated snapshot reflecting required event parameters where optional: false. |
| test-packages/openui5-snapshot-test/output-dts/sap.m.d.ts | Regenerated snapshot reflecting required event parameters where optional: false. |
| test-packages/openui5-snapshot-test/output-dts/sap.f.d.ts | Regenerated snapshot reflecting required event parameters where optional: false. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously, all properties in generated $Xxx$YyyEventParameters interfaces were unconditionally forced to optional (?), regardless of their actual "optional" value in the api.json source data. Now the generator respects each parameter's optional field by default: - optional: false → required (no ?) - optional: true or undefined → optional (?) For events whose parameter optionality has not yet been verified in the OpenUI5/SAPUI5 sources, a new "eventsWithAllParamsOptional" directive (string[] in .dtsgenrc) preserves the legacy all-optional behavior. Format: "fully.qualified.ClassName:eventName" Changes: - generate-from-objects.ts: Add eventsWithAllParamsOptional to Directives. - generate.ts: Initialize the new directive field. - json-fixer.ts: Pass directives to addEventParameterInterfaces(). Add "events" to mapLikeProperties so .dtsgenrc overlays can target individual events by name. - json-event-parameter-interfaces.ts: Invert buildProperties() to respect source data by default; only force all-optional when the event is in the eventsWithAllParamsOptional deny-list. Log a warning when a parameter has no "optional" value (unexpected edge case). - Regenerated snapshot .d.ts files (parameters with optional: false now appear as required). Needed for #529
akudev
force-pushed
the
fix/event-parameters2
branch
from
56ace7a to
0569e6c
Compare
codeworrior
approved these changes
May 20, 2026
Member
codeworrior
left a comment
codeworrior
left a comment
There was a problem hiding this comment.
The default for optional surprises me, but as it is always set, I don't really mind. Besides that, the change looks reasonable to me.
|
|
||
| // All parameters should have optional: true or false in practice; | ||
| // this test documents the defensive fallback behavior (treated as optional). | ||
| it("parameters without optional field default to optional", async () => { |
Member
There was a problem hiding this comment.
I didn't expect this default, see above.
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
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.
Previously, all properties in generated $Xxx$YyyEventParameters interfaces were unconditionally forced to optional (?), regardless of their actual "optional" value in the api.json source data.
Now the generator respects each parameter's optional field by default:
For events whose parameter optionality has not yet been verified in the OpenUI5/SAPUI5 sources, a new "eventsWithAllParamsOptional" directive (string[] in .dtsgenrc) preserves the legacy all-optional behavior. Format: "fully.qualified.ClassName:eventName"
Changes:
generate-from-objects.ts: Add eventsWithAllParamsOptional to Directives.
generate.ts: Initialize the new directive field.
json-fixer.ts: Pass directives to addEventParameterInterfaces(). Add "events" to mapLikeProperties so .dtsgenrc overlays can target individual events by name.
json-event-parameter-interfaces.ts: Invert buildProperties() to respect source data by default; only force all-optional when the event is in the eventsWithAllParamsOptional deny-list. Log a warning when a parameter has no "optional" value (unexpected edge case).
Regenerated snapshot .d.ts files (parameters with optional: false now appear as required).
Needed for #529