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

Add App interface #727

Closed
wants to merge 1 commit into from
Closed

Add App interface #727

wants to merge 1 commit into from

Conversation

joseluisq
Copy link

@joseluisq joseluisq commented Jul 9, 2018

This PR proposes to add an App Typescript interface, which we will allow us to add type checking of some app() function as param. For example if we use hyperapp/render package.

This is motivated by this hyperapp/render PR by @frenzzy that it needs an interface to check the app() function param.

E.g:

import { h, app, App, ActionsType, View } from "hyperapp"
import { withRender, Render } from "@hyperapp/render/node"

interface MyState {
  count: number
}

interface MyActions {
  up(value: number): MyState
}

const state: MyState = {
  count: 0
}

const actions: ActionsType<MyState, MyActions> = {
  up: (value: number) => state => ({
    count: state.count + value
  })
}

const view: View<MyState, MyActions> = (state, actions) => (
  <main>
    <button onclick={actions.up}>{state.count}</button>
  </main>
)

// here withRender() needs to check the app() function param via App interface
const myRender = withRender<App<MyState, MyActions>, Render<MyState, MyActions>>(app)(
  state,
  actions,
  view,
  document.body
)

console.log( myRender.toString() )

Ref: kriasoft/hyperapp-render#10

@codecov
Copy link

codecov bot commented Jul 9, 2018

Codecov Report

Merging #727 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #727   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         166    166           
  Branches       52     52           
=====================================
  Hits          166    166

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebe173a...20e12d0. Read the comment docs.

@jorgebucaran
Copy link
Owner

@joseluisq Can you create a PR for the V1 branch? I plan to use master for 2.0 after I merge the V2 branch.

@jorgebucaran jorgebucaran added the types Strings, numbers, booleans label Jul 9, 2018
@frenzzy frenzzy changed the base branch from master to V1 July 9, 2018 21:17
@okwolf
Copy link
Contributor

okwolf commented Jul 10, 2018

@jorgebucaran looks like the base branch was changed to V1 without having to close the PR.

@jorgebucaran
Copy link
Owner

@okwolf How?

@okwolf
Copy link
Contributor

okwolf commented Jul 10, 2018

@jorgebucaran How?

screen shot 2018-07-09 at 18 01 33

@jorgebucaran
Copy link
Owner

🤯

@joseluisq
Copy link
Author

@joseluisq Can you create a PR for the V1 branch? I plan to use master for 2.0 after I merge the V2 branch.

@jorgebucaran great!
I see that PR was addressed to branch v1.

@frenzzy
Copy link
Contributor

frenzzy commented Jul 10, 2018

What we are waiting for here? Review by @Swizz @Mytrill @dancespiele @pspeter3 ?

@alber70g
Copy link
Contributor

alber70g commented Aug 2, 2018

Why no-one pinged me 😞? The return type for app() is incorrect... It doesn't return Actions but rather WiredActions

@Swizz
Copy link
Contributor

Swizz commented Aug 16, 2018

@alber70g Actions is the right returns type.

The un-wired actions are called ActionsType. I dont remember the why.

@joseluisq
Copy link
Author

joseluisq commented Aug 16, 2018

@alber70g I think that right return type could be something like:

export function app<State, Actions>(
  state: State,
- actions: ActionsType<State, Actions>,
+ actions: Actions,
  view: View<State, Actions>,
  container: Element | null
- ): Actions;
+ ): ActionsType<State, Actions>;

That means the same for App interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types Strings, numbers, booleans
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants