Skip to content

Add abitity to type an accessory context using generics #2664

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 11, 2020
Merged

Add abitity to type an accessory context using generics #2664

merged 1 commit into from
Sep 11, 2020

Conversation

mgcrea
Copy link
Contributor

@mgcrea mgcrea commented Sep 1, 2020 •
edited
Loading

For platform with many accessories leveraging an uniform context, it a bit cumbersome to always cast the proper typing:

eg. const {deviceId, endpointId} = context as TydomAccessoryContext;

This PR adds an optional type generic (without breaking changes) so that you can do something like below where you don't have to re-cast the types.

export const setupContactSensor = (accessory: PlatformAccessory<TydomAccessoryContext>, controller: TydomController): void => {
  const {context} = accessory;
  const {deviceId, endpointId} = context;

@oznu oznu requested a review from bauer-andreas September 2, 2020 05:58
@bauer-andreas
Copy link
Member

Does changing from type any to type unknown for the value of the context object break anything?

@mgcrea
Copy link
Contributor Author

mgcrea commented Sep 2, 2020 •
edited
Loading

I don't think there is, unknown is a better any regarding type-safety but works the same way. Only breaking change is that you can't assign an unknown typed value to another type (without casting with as), while with any there was no type-safety at all (more details). Since the Context type wasn't even exported before I don't think anyone would notice the change.

I you want to err on the cautious side, I can update the PR and switch back to any.

@bauer-andreas
Copy link
Member

I you want to err on the cautious side, I can update the PR and switch back to any.

Nah, I would prefer the change anyways. Just checking if we need to put some breaking change information into the changelog for the devs. Doesn’t impact any running systems anyways. Only compile time issues could potentially arise.

@bauer-andreas bauer-andreas changed the base branch from master to beta September 11, 2020 17:07
@bauer-andreas
Copy link
Member

scheduled changed. This feature will be pulled into v1.2.0
Including it in the current beta now.

@bauer-andreas bauer-andreas merged commit a42f9a8 into homebridge:beta Sep 11, 2020
@bauer-andreas
Copy link
Member

now released with v1.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bauer-andreas bauer-andreas bauer-andreas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mgcrea @bauer-andreas