Skip to content

Conversation

soda-sorcery
Copy link

Description

This PR started as a fix for this TypeScript issue. To handle it, I needed to update how the emitter.ts handled generating interfaces.

The HTMLElement extends the GlobalEventHandlers.onerror property. I didn't want to blow away the whole type to handle this fix. I thought something more flexible would make fixes like this scalable.
I was looking at

const extendConflictsBaseTypes: Record<
  string,
  { extendType: string[]; memberNames: Set<string> }
> = {
  HTMLCollection: {
    extendType: ["HTMLFormControlsCollection"],
    memberNames: new Set(["namedItem"]),
  },
};

and thought that is where I could add the flexibity by adding a new key but I didn't want to break any existing functionality or misuse a type.

I added

const extendConflictsInterfaces: Record<
  string,
  { overrideMap: Record<string, string> }
> = {
  HTMLElement: {
    overrideMap: {
      GlobalEventHandlers: 'Omit<GlobalEventHandlers, "onerror">',
    },
  },
};

It allows you to define a type and override its interfaces when unique cases arise.

I've included fixes for HTMLElement and SvgElement. If these changes are approved but we'd like to split the PR up into discrete chunks that's no problem. I'm also open to other approaches so if there's a more obvious place to bring this in I can update the PR.

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.

@soda-sorcery
Copy link
Author

@microsoft-github-policy-service agree

@saschanaz
Copy link
Contributor

saschanaz commented Sep 14, 2025

Where did UIEvent come from? Looks like a bug.

I think we should instead add an interface that inherits from GlobalEventHandlers (maybe name it DocumentOrElementEventHandlers or such) and make others inherit from it, that way we don't need to repeat onerror for each interface and no need to touch emitter logic (as overridingTypes.jsonc should be able to handle it).

@soda-sorcery
Copy link
Author

That's a good call. Let me see what I can do there.

I have a question out curiosity: is there ever a situation where using something like that omit in the emitter would be necessary or was I misunderstanding some of the architecture of overridingTypes.jsonc?

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.

2 participants