fix: updated CustomEvent.detail to be null when not defined

[GauBen]

Fixes #1585

[github-actions[bot]]

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.

[GauBen]

I'll update the tests accordingly if this change is accepted

[saschanaz]

Sounds fine to me.

[saschanaz]

Can you perhaps also add unit tests for this?

[GauBen]

Is eventlistener.ts the right place for it?

[saschanaz]

I'd prefer a separate file for custom events.

[GauBen]

I just noticed that it is possible to create broken CustomEvents: https://www.typescriptlang.org/play?#code/MYewdgziA2CmB00QHMAUZYHcAEBhArhAC4gC2AogG6xhEA8xATgJZjIB8qA5LNbVwEp4AE1hEAhs2jYAvHOxh80aAIDcAKAD0m7Lr36Dho8aMA9APxA

Do I try to address this too? Would this be a breaking change?

Edit, suggested change:

type CustomEventInit<T = any> = EventInit & (T extends {} ? { detail: T } : { detail?: T });

declare var CustomEvent: {
  prototype: CustomEvent;
  new <T>(
    ...args: T extends {}
      ? [type: string, eventInitDict: CustomEventInit<T>]
      : [type: string, eventInitDict?: CustomEventInit<T>]
  ): CustomEvent<T>;
};

[saschanaz]

Looks right, although it starts to be over-complex... I'm okay with that if @sandersn says okay

[sandersn]

I don't know CustomEvent, but running the example from the playground link, it looks like CustomEvent.default is null if CustomEventInit isn't provided to the constructor. At least, it doesn't appear to be undefined the way that { detail?: T } implies. So what about this?

interface CustomEventInit<T = any> extends EventInit {
    detail: T; // note: you probably need to add the `T extends {} ? T : null` conditional type to map undefined -> null
}

declare var CustomEvent: {
  prototype: CustomEvent;
  new<T>(type: string, eventInitDict: CustomEventInit<T>): CustomEvent<T>
  new(type: string): CustomEvent<null>
};

Of course this might break backward compatibility. I don't know how new or widely-used CustomEvent is.