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

Improve type definitions to allow strict type-checking of ctx.call, actions definitions and events #829

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Telokis
Copy link

@Telokis Telokis commented Oct 17, 2020

📝 Description

The current type definitions do not provide any type-safety regarding ctx.call, ctx.emit, actions definitions in services and events handlers in services.
This merge requests updates the type definitions with type-safety mecanisms for both actions and events.
A detailed explanation can be found in #822 where the implementation was explained as it was developed.

The changes are made with 100% retro-compatibility in mind. If no mapping is provided, the behavior is the current, forgiving one.

Impacts of the type definitions changes:

  • Action handlers must return the proper value if ActionsMapping and ActionsEntry are specified.
  • Field actions of service schema recognizes specific keys based on ActionsMapping and ActionsEntry.
  • Field events of service schema recognizes specific keys based on EventsMapping.
  • never can be used as first generic argument for Service. (Fallbacks to the default value)
  • Context.params is strictly-typed within an action handler if ActionsMapping is provided.
  • Context.params is strictly-typed within an event handler if EventsMapping is provided.
  • Context.eventName is strictly-typed if EventsMapping is provided.
  • Context.call is strictly-typed if ActionsMapping is provided.
  • Context.emit is strictly-typed if EventsMapping is provided.
  • Context.broadcast is strictly-typed if EventsMapping is provided.
  • ServiceBroker.call is strictly-typed if ActionsMapping is provided.
  • ServiceBroker.emit is strictly-typed if EventsMapping is provided.
  • ServiceBroker.broadcast is strictly-typed if EventsMapping is provided.
  • ServiceBroker.broadcastLocal is strictly-typed if EventsMapping is provided.
  • The index signature of ActionSchema has been narrowed down a bit. See this comment and the section Caveats at the bottom of this description.

🎯 Relevant issues

Closes #822

💎 Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

📜 Example code

Complete example in TypeSript playground

🚦 How Has This Been Tested?

This is currently being used in the project I made the changes for.
The playground link provided above also contains a quite exhaustive example of the new type-safety.

🏁 Checklist:

  • My code follows the style guidelines of this project
    --> I reverted everything my eslint/prettier changed.
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
    --> The tsd dependency doesn't support TypeScript 4.1.0 yet. Tests are broken until then.
  • New and existing unit tests pass locally with my changes
    --> Same reason as above.
  • I have commented my code, particularly in hard-to-understand areas
    --> I did my best to make sure every trick is properly commented and explained

⚠️ Caveats:

  • The code requires TypeScript 4.1.0 (Heavily relies on this feature) and can't be merged until it is officially released.
  • The too permissive type mentionned in this comment
  • The ServiceEventLegacyHandler/ServiceEventHandler ambiguity mentioned in this comment below ServiceEvent function deduction issue.
  • People might rely on the syntax using *s for event handling. Those aren't recognized as special cases and must be @ts-ignored or specified in the EventsMapping. (The latter is recommended)

@Telokis Telokis marked this pull request as draft October 17, 2020 17:37
@Wallacy
Copy link
Contributor

Wallacy commented Dec 22, 2020

@Telokis Telokis mentioned this pull request Feb 26, 2021
4 tasks
@Telokis
Copy link
Author

Telokis commented Feb 26, 2021

@Wallacy Hi! Yes, I noticed the release, thanks for telling me.
For now, I'm a bit burned out, I don't know when I'll be able to take the time and finish this properly.

@marbemac
Copy link

marbemac commented Oct 7, 2021

@icebob we ended up forking off of this and improving a few more typing related things - it has been immensely helpful, thank you @Telokis! Would have to double check but I don't think there are any breaking changes - if that's the case is this sort of typing change something you would consider accepting? If so we'll put it on our list to open up a PR.

@icebob
Copy link
Member

icebob commented Oct 11, 2021

@marbemac if it contains big changes which can cause problems for developers who are using the current typescript definitions, that I will be able to accept the PR in the next 0.15 version.

@edurdias
Copy link

edurdias commented Oct 11, 2021 via email

@icebob
Copy link
Member

icebob commented Oct 11, 2021

@edurdias not yet. I'm collecting the major features/changes what will be implemented in 0.15. I think it can be released only in 22H1.

@Laurin-W
Copy link

Are there any updates on this? Would be super helpful.. :)

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.

Improve type definitions to allow strict type-checking of ctx.call, actions definitions and events
6 participants