Skip to content
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

feat: Typing for MQTT API #25098

Merged
merged 2 commits into from
Dec 7, 2024
Merged

feat: Typing for MQTT API #25098

merged 2 commits into from
Dec 7, 2024

Conversation

Nerivec
Copy link
Collaborator

@Nerivec Nerivec commented Dec 6, 2024

  • Types for the entire MQTT API
    • Some currently unused to avoid too much refactoring in current PR but available for future (tried to minimize actual code changes in this one)
    • Exported for easy access by 3rd parties using package
    • Should eventually allow to create a script for docs that automatically parses the interface and updates the doc page
  • Remove small request lookups (externalJS, networkMap) in favor of plain calls for better typing
  • Fix configure payload validation (+test)
  • Fix external JS save/remove payload validation (+tests)
  • Remove unused global types

TODO:

  • Remove request lookup for bridge for better typing too (switch statement instead)?
  • Some typing is not possible/ideal due to current handling (can progressively refactor).
  • HA extension not typed. Too much dynamic/inferred, probably will require special handling...

@Koenkk Koenkk enabled auto-merge (squash) December 7, 2024 20:20
@Koenkk
Copy link
Owner

Koenkk commented Dec 7, 2024

Thanks!

@Koenkk Koenkk merged commit 1a9c79b into Koenkk:dev Dec 7, 2024
11 checks passed
@Nerivec Nerivec deleted the mqtt-api-typing branch December 7, 2024 22:36
@Nerivec
Copy link
Collaborator Author

Nerivec commented Dec 7, 2024

For future refactoring, I'm thinking we should probably have something like this.mqtt.publishResponse that absorbs both the utils.getResponse and stringify, and allows proper typing based on API endpoint. It probably would also allow short-circuiting some of the logic in this.mqtt.publish that never applies for responses.

Some places are still using "old JS", where no-typing allowed many possible uses at once, we'll likely have to refactor that too (like bridge.ts). That should progressively remove the dependency on KeyValue and KeyValueAny types, at the cost of a few more lines of code to ensure proper type-checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants