Potential fix for Event.raiseEvent() typescript

[chris-cooper]

I think this may be a fix for the Event.raiseEvent() type, as Parameters<Listener> is already a spread ...args: any[]. It looks like the additional spread operator is turning it into something like an ...any[][].

i.e. Listener extends (...args: any[]) => void so Parameters<Listener> would be an (...any[])

Issue: https://github.com/CesiumGS/cesium/issues/10498

    // before
    raiseEvent(...arguments: Parameters<Listener>[]): void;

    // after
    raiseEvent(arguments: Parameters<Listener>): void;

[cesium-concierge]

Thanks for the pull request @chris-cooper!

• :heavy_check_mark: Signed CLA found.
• :grey_question: CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• :grey_question: Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• [ ] Cesium Viewer works.
• [ ] Works in 2D/CV.

[ggetz]

Thanks @chris-cooper! @thw0rted any thoughts on this fix?

[thw0rted]

I pulled this branch and ran npm build-ts. The resulting types look like

    /**
     * Raises the event by calling each registered listener with all supplied arguments.
     * @param arguments - This method takes any number of parameters and passes them through to the listener functions.
     */
    raiseEvent(arguments: Parameters<Listener>): void;

which is still wrong. It should be raiseEvent(...arguments: Parameters<Listener>): void;. The wrong output means the function takes one argument, which is an array. The desired output, with the spread-operator, means the function takes exactly the same number and type of arguments as Listener does -- that is, the rest-parameter of the function is a tuple.

I don't think tsd-jsdoc is capable of generating the syntax needed to strongly type this function right now. I asked on their tracker and have not received a response. I asked the TS team to clarify, and that discussion is still ongoing. It could get fixed eventually but I'm not holding my breath. (Sorry!)

[thw0rted]

I've just come up with a slightly terrible workaround, that I think actually kinda-sorta fixes it: https://github.com/thw0rted/cesium/tree/raiseEvent-fix

This manually replaces the invalid TS output during the Gulp post-processing phase. I've used this crutch in the past to work around things that tsd-jsdoc can't quite do. It's brittle, but hopefully if it breaks in the future, the simple tests I added will break and someone will notice.

[chris-cooper]

Ok thanks @thw0rted . I've tested your branch on our code and it allows removal of the @ts-expect-error s that were introduced with 1.95. I think it would be preferable to incorporate either your workaround, or even a rollback to a more loosely typed raiseEvent() rather than imposing the incorrect type (from 1.95).

[chris-cooper]

I've cherry-picked aee2bbfe135f77514c96170c877bce9907d81f0e and merged master in case you want to proceed with this PR, or feel free to close and open a fresh one @thw0rted .

[cesium-concierge]

Thanks again for your contribution @chris-cooper!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @chris-cooper!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @chris-cooper!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @chris-cooper!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.