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 Types for $delegate and $resolve function #1567

Open
wants to merge 6 commits into
base: v4/latest
Choose a base branch
from

Conversation

jBernavaPrah
Copy link

@jBernavaPrah jBernavaPrah commented Jul 3, 2023

Proposed Changes

Added a new abstract Class BaseDelegateComponent and an ExtractDelegatedEventData type to permit better use of $delegate and $resolve functions.

The BaseDelegateComponent, used in a similar way as the BaseComponent, is there to add the typing to events/resolve.
The ExtractDelegatedEventData is used to extract the data from the event.

Examples (pseudo code):

// components/DelegatedComponent.ts
class DelegatedComponent extends BaseDelegatedComponent<{
  completed: boolean, // completed is the event, boolean is the parameter, for multiple params, use array like: [boolean, string, string[]]
}> {
 async START() {
   return this.$resolve('completed', true) // <-- Typed
   
  //return this.$resolve('wrongEvent', "orWrongType")  <-- Error type from typescript
 }

}

// components/GlobalComponent
class GlobalComponent extends BaseComponent {

  async intent(){
    this.delegate(DelegatedComponent, {
    resolve: {
          completed: this.onDelegateCompleted // completed will be suggested by IDE
          // For back-compatibility, adding something wrong will NOT result in an error. We can discuss it if we think this can be useful.
    }
    })
  }
  
  onDelegateCompleted(result: ExtractDelegatedEventData<DelegatedComponent, 'completed'>){
  //  result is typed  
  if(result) return this.$send("Result frue");
  return this.$send("Result false");
  }

}

Completely open to discussing it in any way.

Types of Changes

  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • [-] I have added tests to cover my changes
  • All new and existing tests passed

Added a *breaking change* in Jovo.$resolve typing. I'm try to find a better way without the necessity to change the type of ARGS, but for now it's working for the delegateComponent.
@jBernavaPrah
Copy link
Author

Update the PR with a fix to improve the typing when used with arrays of type.

This last change introduces a breaking change because of the generic ARGS of Jovo.$resolve needs to be compatible with a tuple and not with an array.

So now, if users use only BaseComponent, typescript will trigger errors when multiple parameters will be used in the $resolve. Es:

this.$resolve('event', "resolve1", "resolve2") // The resolve2 will be highlight by typescript as error

Happy to improve and remove this breaking change if someone has some suggestions!

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.

1 participant