⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@kairosci
Copy link
Contributor

@kairosci kairosci commented Jan 14, 2026

Description

This PR updates the emitter logic to consistently handle optional dictionary members by explicitly allowing undefined in their type definitions. Following the maintainer's feedback, I have shifted from a specific fix for WebCodecs to a generalized emitter change, avoiding manual KDL patches or redundant entries in overridingTypes.jsonc.

Key Changes

  1. Generalized Emitter Logic: Modified src/build/emitter.ts to automatically append | undefined to all optional dictionary members during the emission process.
  2. Smart Deduplication: Implemented a check within the emitter to ensure that | undefined is only added if not already present in the type union (e.g., preventing cases like type?: undefined | undefined).
  3. Removal of Redundant Overrides: Removed all manual | undefined overrides for dictionary members from overridingTypes.jsonc, as they are now handled natively by the generator.
  4. Baseline Synchronization: Regenerated all baseline files against the latest main branch to resolve conflicts and ensure consistency across all supported TypeScript versions.

Why this is important?

  • Consistency: Interface properties in this project already follow the prop?: T | undefined pattern. This change brings Dictionaries (configuration objects) into alignment with that standard.
  • Support for exactOptionalPropertyTypes: In strict TypeScript configurations where exactOptionalPropertyTypes is enabled, a property defined as prop?: T cannot be explicitly assigned undefined. By changing this to prop?: T | undefined, we allow developers to explicitly pass undefined to configuration dictionaries, which is a common and often necessary pattern in Web APIs.
  • Alignment with WebIDL Semantics: WebIDL optionality often implies that undefined is a valid value. This change accurately reflects those semantics in the generated TypeScript definitions.
  • Maintainability: By centralizing this logic in the emitter rather than using KDL patches or JSON overrides, we keep the codebase cleaner and reduce the risk of future regressions.

Implementation Details

The PR is organized into two functional commits:

  • feat(emitter): generalize optional dictionary members to allow explicit undefined: Contains the core logic changes in the generator.
  • refactor(overrides): update baselines with generalized optional dictionary members: Contains the baseline updates, fully synced with the current state of the upstream repository.

@github-actions
Copy link
Contributor

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

@kairosci kairosci changed the title Allow explicit undefined for optional WebCodecs config properties (#1557) Allow explicit undefined for optional WebCodecs config properties Jan 14, 2026
Copy link
Contributor

@Bashamega Bashamega left a comment

Choose a reason for hiding this comment

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

Please use KDL, and DO NOT us OverrideType

@saschanaz
Copy link
Collaborator

saschanaz commented Jan 14, 2026

This should not be webcodecs specific but be a general emitter change. KDL should not be used either.

@kairosci kairosci changed the title Allow explicit undefined for optional WebCodecs config properties feat(emitter): generalize optional dictionary members to allow explicit undefined Jan 14, 2026
@kairosci
Copy link
Contributor Author

This should not be webcodecs specific but be a general emitter change. KDL should not be used either.

I have updated this PR to address your feedback.

I have shifted the implementation from a WebCodecs-specific fix to a generalized emitter change in src/build/emitter.ts.

The generator now automatically appends | undefined to all optional dictionary members, ensuring consistency with how interface properties are handled.

This approach eliminates the need for KDL patches or manual overrides in overridingTypes.jsonc, which I have cleaned up.

I have also synchronized all baselines with the latest main branch and organized the work into two clean, functional commits. This change should correctly support exactOptionalPropertyTypes for all configuration dictionaries across the board.

@kairosci kairosci requested a review from Bashamega January 14, 2026 10:24
Comment on lines 1583 to 1584
let type = convertDomTypeToTsType(m);
if (!m.required && !type.split(" | ").includes("undefined")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably pass a modified type to the conversion function rather than modifying the stringified result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I have satisfied this change too.

Now there are no more concatenations, but the management of | undefined, is handled natively, passing it directly to the conversion function.

@kairosci kairosci requested a review from saschanaz January 14, 2026 11:25
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.

3 participants