-
-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optional AllowedEvents/AllowedActions #446
Optional AllowedEvents/AllowedActions #446
Conversation
The restricted controller messenger now allows omitting allowedActions and allowedEvents. Technically it already allowed this, but it required setting the type of the allowed actions/events to an empty string. Now the type is `never` if no actions/events are used.
c81ceaf
to
7c7eb66
Compare
src/ControllerMessenger.ts
Outdated
@@ -82,13 +82,13 @@ export class RestrictedControllerMessenger< | |||
}: { | |||
controllerMessenger: ControllerMessenger<Action, Event>; | |||
name: N; | |||
allowedActions: AllowedAction[]; | |||
allowedEvents: AllowedEvent[]; | |||
allowedActions?: AllowedAction[] | never; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per discussion, with ?:
, is | never
necessary or vice versa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?: allows for null or undefined.
I believe this is stating that if it is defined it must be an AllowedEvent[]
or it'll throw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds reasonable to me 👍 I wasn't entirely sure of the exact behavior with never
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving the | never
out also serves to ensure that this parameter must be an AllowedEvent[]
though. I think it's superfluous.
I propagated it here because I used it in getRestrictedControllerMessenger
, where I needed the type for AllowedEvents
to be something because optional generic parameters aren't a thing that exists. So I need the | never
for the generic parameters, but I'll remove them for the optional parameters (here and in getRestrictedControllerMessenger
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
The restricted controller messenger now allows omitting allowedActions and allowedEvents. Technically it already allowed this, but it required setting the type of the allowed actions/events to an empty string. Now the type is `never` if no actions/events are used.
The restricted controller messenger now allows omitting allowedActions and allowedEvents. Technically it already allowed this, but it required setting the type of the allowed actions/events to an empty string. Now the type is `never` if no actions/events are used.
The restricted controller messenger now allows omitting allowedActions and allowedEvents. Technically it already allowed this, but it required setting the type of the allowed actions/events to an empty string. Now the type is
never
if no actions/events are used.