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

Simplify generics for IContextContainer #46538

Merged
merged 7 commits into from
Oct 3, 2019

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Sep 24, 2019

Summary

This greatly simplifies the generic types for the IContextContainer by leveraging more type inference and conditional types to be able to pull out all the necessary types from a single generic.

Before

Before this change you had to separately define the type of the context object, the return type of the handlers, and any additional arguments provided to handlers.

core.context.createContextContainer<
  AppMountContext,
  AppUnmount | Promise<AppUnmount>,
  [AppMountParameters]
>();

After

Now, all you need to provide is the signature of the handlers:

core.context.createContextContainer<
  (
    context: AppMountContext,
    params: AppMountParameters
  ) => AppUnmount | Promise<AppUnmount>
>();

This can be simplified even further if you already have a type defined for this:

core.context.createContextContainer<App['mount']>();

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover marked this pull request as ready for review September 25, 2019 14:15
@joshdover joshdover requested a review from a team as a code owner September 25, 2019 14:15
@joshdover joshdover added Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.5.0 labels Sep 25, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@joshdover
Copy link
Contributor Author

@elasticmachine merge upstream

@mshustov
Copy link
Contributor

ack will review tomorrow

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover merged commit 5fc9a55 into elastic:master Oct 3, 2019
@joshdover joshdover deleted the np/context-types branch October 3, 2019 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants