Skip to content

add plugin types to ctx interface#579

Merged
nicolodavis merged 9 commits into
boardgameio:masterfrom
janKir:janKir/ctx-enhanced-types
Mar 27, 2020
Merged

add plugin types to ctx interface#579
nicolodavis merged 9 commits into
boardgameio:masterfrom
janKir:janKir/ctx-enhanced-types

Conversation

@janKir
Copy link
Copy Markdown
Contributor

@janKir janKir commented Mar 25, 2020

Checklist

  • Use a separate branch in your local repo (not master).
  • Test coverage is 100% (or you have a story for why it's ok).

When using the typings of Ctx I noticed it was missing the events. After having a look into the source code, I realized the random and player plugins also enhance the Ctx interface.

The name plugin implies that they are not necessarily defined, so I declared them optional.
Also it seems like specific event functions can be deactivated, so I declared those functions all optional as well. This has the downside, that you need to add checks or non-null assertions to your typescript code when calling those event functions.

@googlebot
Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@janKir
Copy link
Copy Markdown
Contributor Author

janKir commented Mar 25, 2020

@googlebot I signed it!

@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Copy Markdown
Member

@nicolodavis nicolodavis left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment thread src/types.ts Outdated
@delucis
Copy link
Copy Markdown
Member

delucis commented Mar 25, 2020

I think it might be a good idea to make some of the types generics, so that users can do things like:

type CtxCopy = Ctx; // has a default value with default plugins enabled 
type CustomCtx = Ctx<{}>; // has no plugins enabled
type ExtendedCtx = Ctx<DefaultPlugins & { x: value }>; // extends default

Similarly for State, which could support arguments for G and Ctx:

// CustomState['G'] will have type MyG instead of any
type CustomState = State<MyG, CustomCtx>;

It should then be possible for users to do something like the following and type check as expected:

type MyG = { val: boolean };

const myGame: GameConfig<MyG, CustomCtx> = {
  endIf: G => G.field, // error: field does not exist in type MyG
};

I need to check how would be best to implement this, but the types for Koa work like this.

@janKir
Copy link
Copy Markdown
Contributor Author

janKir commented Mar 25, 2020

@delucis I think this should work for the Ctx type:

export interface CtxBase {
  numPlayers: number;
  playOrder: Array<PlayerID>;
  playOrderPos: number;
  activePlayers: null | ActivePlayers;
  currentPlayer: PlayerID;
  numMoves?: number;
  gameover?: any;
  turn: number;
  phase: string;
  _activePlayersMoveLimit?: object;
  _activePlayersNumMoves?: object;
  _prevActivePlayers?: Array<object>;
  _random?: {
    seed: string | number;
  };
}

export type Ctx<
  T = { events: EventsAPI; random: RandomAPI }
> = CtxBase & T;

However, I guess this will break typing inside boardgame.io where the Ctx interface is already used for objects without those plugins. Instead of Ctx it would be necessary to use CtxBase or Ctx<{}> there.

Edit: Realized that the player Plugin is not active by default, so I removed it.

Comment thread src/plugins/plugin-player.ts
@delucis
Copy link
Copy Markdown
Member

delucis commented Mar 25, 2020

I guess this will break typing inside boardgame.io where the Ctx interface is already used for objects without those plugins. Instead of Ctx it would be necessary to use CtxBase or Ctx<{}> there.

Yes, it’ll need a bit of reorganising. The Ctx type should probably be the one without plugins and we can use a CtxWithPlugins internally and expose something else to users:

type Ctx<Plugins extends { [plugin: string]: any } = {}> = {
  numPlayers: number;
  // etc.
} & Plugins;

type DefaultPlugins = {
  events: EventsAPI;
  random: RandomAPI;
};

type CtxWithPlugins = Ctx<DefaultPlugins>;

It probably still needs some wider consideration to work out how to cascade the types down to all the bits of code that need them.

@janKir
Copy link
Copy Markdown
Contributor Author

janKir commented Mar 25, 2020

It probably still needs some wider consideration to work out how to cascade the types down to all the bits of code that need them.

Maybe keep the changes simple for this PR and work on that on a follow-up PR?

@delucis
Copy link
Copy Markdown
Member

delucis commented Mar 25, 2020

Maybe keep the changes simple for this PR and work on that on a follow-up PR?

Sounds good to me!

@nicolodavis nicolodavis merged commit 167690c into boardgameio:master Mar 27, 2020
pastpatryk pushed a commit to mavend/boardgame.io that referenced this pull request Apr 7, 2020
* add plugin types to ctx interface

* extract plugin interfaces into corresponding plugin files

* add missing state field in PlayerAPI interface

* fix type errors in player plugin api()

* use events return type void

Co-authored-by: Nicolo John Davis <[email protected]>
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.

4 participants