fix(plugins): keep plugin event maps under exactOptionalPropertyTypes - #239
Merged
Conversation
The `__pluginEvents` marker is a phantom, so it is declared optional and
the emitted declaration writes it as `?: E | undefined`. A consumer
compiling with `exactOptionalPropertyTypes` keeps that `undefined` when
`EventsOf` infers `E`, and `UnionToIntersection` then reduces
`Events | undefined` to `never`. The client resolves as
`WaClient<never>`, which types every listener parameter as `never`,
including core events that have nothing to do with a plugin:
client.on('auth_qr', ({ qr }) => ...)
// Argument of type '...' is not assignable to parameter of type 'never'
// Binding element 'qr' implicitly has an 'any' type
`EventsOf` now drops `undefined` before the union reaches the
intersection. Every plugin was affected, not only the one reported:
voip fails identically despite declaring a real event map, since the
collapse happens after the marker is read, not because of what it holds.
The break only reproduces through the built .d.ts. Compiling against
the sources infers `E` from an optional property with no explicit
`| undefined`, which the flag has nothing to preserve, so it cannot be
covered from `src/`: an assertion there passes with and without this
change. A real guard needs a fixture compiled against `dist/` with the
flag on.
Verified against the reporter's dependency set (zapo-js 1.7.0,
@zapo-js/wam 0.1.1, @zapo-js/voip) with the flag on: core events,
plugin getters and plugin events all type, and an unknown event name
is still rejected.
Reported in #236.
📝 WalkthroughWalkthrough
ChangesPlugin event type handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
The
__pluginEventsmarker is a phantom, so it is declared optional and the emitted declaration writes it as?: E | undefined. A consumer compiling withexactOptionalPropertyTypeskeeps thatundefinedwhenEventsOfinfersE, andUnionToIntersectionthen reducesEvents | undefinedtonever. The client resolves asWaClient<never>, which types every listener parameter asnever, including core events that have nothing to do with a plugin:EventsOfnow dropsundefinedbefore the union reaches the intersection. Every plugin was affected, not only the one reported: voip fails identically despite declaring a real event map, since the collapse happens after the marker is read, not because of what it holds.The break only reproduces through the built .d.ts. Compiling against the sources infers
Efrom an optional property with no explicit| undefined, which the flag has nothing to preserve, so it cannot be covered fromsrc/: an assertion there passes with and without this change. A real guard needs a fixture compiled againstdist/with the flag on.Verified against the reporter's dependency set (zapo-js 1.7.0, @zapo-js/wam 0.1.1, @zapo-js/voip) with the flag on: core events, plugin getters and plugin events all type, and an unknown event name is still rejected.
Closes #236.
Summary by CodeRabbit
Bug Fixes
Documentation
exactOptionalPropertyTypessetting.