Skip to content

Narrow type of PluginDeps to an object #40846

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

Merged
mshustov merged 4 commits into from
Jul 23, 2019
Merged

Narrow type of PluginDeps to an object #40846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e5386b
Narrow type of PluginDeps to an object
mshustov Jul 11, 2019
f81e11d
re-generate docs
mshustov Jul 11, 2019
a69e5ab
Merge branch 'master' into unify-plugin-deps-interface
mshustov Jul 15, 2019
6e5a05d
Merge branch 'master' into unify-plugin-deps-interface
mshustov Jul 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/development/core/public/kibana-plugin-public.plugin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ The interface that should be returned by a `PluginInitializer`<!-- -->.
<b>Signature:</b>

```typescript
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends {} = {}, TPluginsStart extends {} = {}>
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends object = object, TPluginsStart extends object = object>
```

## Methods
Expand Down
2 changes: 1 addition & 1 deletion docs/development/core/public/kibana-plugin-public.plugininitializer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ The `plugin` export at the root of a plugin's `public` directory should conform
<b>Signature:</b>

```typescript
export declare type PluginInitializer<TSetup, TStart, TPluginsSetup extends Record<string, any> = {}, TPluginsStart extends Record<string, any> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
export declare type PluginInitializer<TSetup, TStart, TPluginsSetup extends object = object, TPluginsStart extends object = object> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
```
2 changes: 1 addition & 1 deletion docs/development/core/server/kibana-plugin-server.plugin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ The interface that should be returned by a `PluginInitializer`<!-- -->.
<b>Signature:</b>

```typescript
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends {} = {}, TPluginsStart extends {} = {}>
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends object = object, TPluginsStart extends object = object>
```

## Methods
Expand Down
2 changes: 1 addition & 1 deletion docs/development/core/server/kibana-plugin-server.plugininitializer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ The `plugin` export at the root of a plugin's `server` directory should conform
<b>Signature:</b>

```typescript
export declare type PluginInitializer<TSetup, TStart, TPluginsSetup extends Record<PluginName, unknown> = {}, TPluginsStart extends Record<PluginName, unknown> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
export declare type PluginInitializer<TSetup, TStart, TPluginsSetup extends object = object, TPluginsStart extends object = object> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
```
14 changes: 7 additions & 7 deletions src/core/public/plugins/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { DiscoveredPlugin, PluginName } from '../../server';
import { DiscoveredPlugin } from '../../server';
import { PluginInitializerContext } from './plugin_context';
import { loadPluginBundle } from './plugin_loader';
import { CoreStart, CoreSetup } from '..';
Expand All @@ -30,8 +30,8 @@ import { CoreStart, CoreSetup } from '..';
export interface Plugin<
TSetup = void,
TStart = void,
TPluginsSetup extends {} = {},
TPluginsStart extends {} = {}
TPluginsSetup extends object = object,
TPluginsStart extends object = object
> {
setup(core: CoreSetup, plugins: TPluginsSetup): TSetup | Promise<TSetup>;
start(core: CoreStart, plugins: TPluginsStart): TStart | Promise<TStart>;
Expand All @@ -47,8 +47,8 @@ export interface Plugin<
export type PluginInitializer<
TSetup,
TStart,
TPluginsSetup extends Record<string, any> = {},
TPluginsStart extends Record<string, any> = {}
TPluginsSetup extends object = object,
TPluginsStart extends object = object
> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;

/**
Expand All @@ -60,8 +60,8 @@ export type PluginInitializer<
export class PluginWrapper<
TSetup = unknown,
TStart = unknown,
TPluginsSetup extends Record<PluginName, any> = Record<PluginName, unknown>,
TPluginsStart extends Record<PluginName, any> = Record<PluginName, unknown>
TPluginsSetup extends object = object,
TPluginsStart extends object = object
> {
public readonly name: DiscoveredPlugin['id'];
public readonly configPath: DiscoveredPlugin['configPath'];
Expand Down
10 changes: 5 additions & 5 deletions src/core/public/plugins/plugin_context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import { omit } from 'lodash';

import { DiscoveredPlugin, PluginName } from '../../server';
import { DiscoveredPlugin } from '../../server';
import { CoreContext } from '../core_system';
import { PluginWrapper } from './plugin';
import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service';
Expand Down Expand Up @@ -61,8 +61,8 @@ export function createPluginInitializerContext(
export function createPluginSetupContext<
TSetup,
TStart,
TPluginsSetup extends Record<PluginName, unknown>,
TPluginsStart extends Record<PluginName, unknown>
TPluginsSetup extends object,
TPluginsStart extends object
>(
coreContext: CoreContext,
deps: PluginsServiceSetupDeps,
Expand All @@ -89,8 +89,8 @@ export function createPluginSetupContext<
export function createPluginStartContext<
TSetup,
TStart,
TPluginsSetup extends Record<PluginName, unknown>,
TPluginsStart extends Record<PluginName, unknown>
TPluginsSetup extends object,
TPluginsStart extends object
>(
coreContext: CoreContext,
deps: PluginsServiceStartDeps,
Expand Down
8 changes: 4 additions & 4 deletions src/core/public/plugins/plugin_loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export const LOAD_TIMEOUT = 120 * 1000; // 2 minutes
export const loadPluginBundle: LoadPluginBundle = <
TSetup,
TStart,
TPluginsSetup extends Record<string, unknown>,
TPluginsStart extends Record<string, unknown>
TPluginsSetup extends object,
TPluginsStart extends object
>(
addBasePath: (path: string) => string,
pluginName: PluginName,
Expand Down Expand Up @@ -125,8 +125,8 @@ export const loadPluginBundle: LoadPluginBundle = <
export type LoadPluginBundle = <
TSetup,
TStart,
TPluginsSetup extends Record<string, unknown>,
TPluginsStart extends Record<string, unknown>
TPluginsSetup extends object,
TPluginsStart extends object
>(
addBasePath: (path: string) => string,
pluginName: PluginName,
Expand Down
4 changes: 2 additions & 2 deletions src/core/public/public.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ export interface OverlayStart {
}

// @public
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends {} = {}, TPluginsStart extends {} = {}> {
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends object = object, TPluginsStart extends object = object> {
// (undocumented)
setup(core: CoreSetup, plugins: TPluginsSetup): TSetup | Promise<TSetup>;
// (undocumented)
Expand All @@ -485,7 +485,7 @@ export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends {} =
}

// @public
export type PluginInitializer<TSetup, TStart, TPluginsSetup extends Record<string, any> = {}, TPluginsStart extends Record<string, any> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
export type PluginInitializer<TSetup, TStart, TPluginsSetup extends object = object, TPluginsStart extends object = object> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;

// @public
export interface PluginInitializerContext {
Expand Down
12 changes: 6 additions & 6 deletions src/core/server/plugins/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ export interface DiscoveredPluginInternal extends DiscoveredPlugin {
export interface Plugin<
TSetup = void,
TStart = void,
TPluginsSetup extends {} = {},
TPluginsStart extends {} = {}
TPluginsSetup extends object = object,
TPluginsStart extends object = object
> {
setup(core: CoreSetup, plugins: TPluginsSetup): TSetup | Promise<TSetup>;
start(core: CoreStart, plugins: TPluginsStart): TStart | Promise<TStart>;
Expand All @@ -155,8 +155,8 @@ export interface Plugin<
export type PluginInitializer<
TSetup,
TStart,
TPluginsSetup extends Record<PluginName, unknown> = {},
TPluginsStart extends Record<PluginName, unknown> = {}
TPluginsSetup extends object = object,
TPluginsStart extends object = object
Copy link
Contributor

@joshdover joshdover Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we change this to TPluginsSetup extends object = Record<string, unknown>? Wouldn't that force the consumer to specify a type rather than defaulting to a type whose values are any?

Copy link
Contributor Author

@mshustov mshustov Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in what cases it could help us?
1 if a plugin implements Plugin interface, it still has to declare all types explicitly, because TS cannot infer contextual types from an interface
2 if an object implements Plugin interface we have the same problem as discussed here. end users have to declare all required members when we fall back to the object type.

There is a sandbox with both examples

Copy link
Contributor

@joshdover joshdover Jul 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I was mistaken on how the raw object type worked. I thought it would default to essentially Record<string, any>. This LGTM 👍

> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;

/**
Expand All @@ -168,8 +168,8 @@ export type PluginInitializer<
export class PluginWrapper<
TSetup = unknown,
TStart = unknown,
TPluginsSetup extends Record<PluginName, unknown> = Record<PluginName, unknown>,
TPluginsStart extends Record<PluginName, unknown> = Record<PluginName, unknown>
TPluginsSetup extends object = object,
TPluginsStart extends object = object
> {
public readonly name: PluginManifest['id'];
public readonly configPath: PluginManifest['configPath'];
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/server.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ export interface OnPreAuthToolkit {
}

// @public
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends {} = {}, TPluginsStart extends {} = {}> {
export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends object = object, TPluginsStart extends object = object> {
// (undocumented)
setup(core: CoreSetup, plugins: TPluginsSetup): TSetup | Promise<TSetup>;
// (undocumented)
Expand All @@ -341,7 +341,7 @@ export interface Plugin<TSetup = void, TStart = void, TPluginsSetup extends {} =
}

// @public
export type PluginInitializer<TSetup, TStart, TPluginsSetup extends Record<PluginName, unknown> = {}, TPluginsStart extends Record<PluginName, unknown> = {}> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;
export type PluginInitializer<TSetup, TStart, TPluginsSetup extends object = object, TPluginsStart extends object = object> = (core: PluginInitializerContext) => Plugin<TSetup, TStart, TPluginsSetup, TPluginsStart>;

// @public
export interface PluginInitializerContext<ConfigSchema = unknown> {
Expand Down