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

Question: does anyone know of a boilerplate generator for ng2-redux? #210

Closed
sobvan opened this issue Aug 27, 2016 · 17 comments
Closed

Question: does anyone know of a boilerplate generator for ng2-redux? #210

sobvan opened this issue Aug 27, 2016 · 17 comments

Comments

@sobvan
Copy link

sobvan commented Aug 27, 2016

Of course, the situation is not that bad, but would be nice to make it even easier to use by generating actions and reducer frames.
Mostly because it is easy to mistype variables, and AFAIK the reducer cannot be used in a fully typed way.

@SethDavenport
Copy link
Member

SethDavenport commented Aug 27, 2016

I'm not aware of a generator, no. It's an interesting idea though.

Can you clarify what you mean about 'using reducers in a fully typed way'?

Redux itself exposes types for actions and reducers. Generally I try to strongly type my reducers as follows:

import { Action, Reducer } from 'redux';

export interface IPayloadAction<P> extends Action {
  payload?: P;
}

export interface IMyState {
  ...
}

export const myReducer: Reducer<IMyState> = (
  state: IMyState = INITIAL_STATE,
  action: IPayloadAction<IPayload>) => {
  // ...
}

@SethDavenport
Copy link
Member

In a multi reducer application, I typically compose my state types too, e.g:

interface IAppState {
  state1?: IState1,
  // ...
  stateN?: IStateN,
}

export const rootReducer: Reducer<IAppState> = combineReducers({
  state1: reducer1,
  // ...
  state2: reducerN,
};

///

constructor(private ngRedux: NgRedux<IAppState>) {}

@SethDavenport
Copy link
Member

This works well for stores made up of plain objects. Getting it to play well with stores made out of constructs from ImmutableJS is a bit trickier because all your state ends up having type Map<string, any>

My colleague @danielfigueiredo has been experimenting with a solution to the problem of stong typings with Immutable.Record and Immutable.Map over here: https://github.com/rangle/typed-immutable-record

@sobvan
Copy link
Author

sobvan commented Aug 28, 2016

Hello @SethDavenport, IPayLoadAction helps a bit indeed. However I do my reducer logic like this:

    case TRIP_ACTIONS.QUERY_TRIPS_ASYNC:
      if (state.loading)
        return state;
      return Object.assign({}, state, {
        loading: true,
        trips: null
      }); 

here the new object

      ...{
        loading: true,
        trips: null
      }

cannot select the variables to set in a typed way.
Do you have some solution for this?

Of course, this part would not be possible to generate, as this is the logic itself.

@SethDavenport
Copy link
Member

SethDavenport commented Aug 30, 2016

Hmm. I guess you're looking for a more strongly typed Object.assign? I can see that being tricky, for sure. My guess is that's the main reason Typescript doesn't support object property spread syntax either (const foo = { prop1: 1, ...someOtherProps }).

This is one of the ways TypedImmutableRecord tries to provide a better experience, by exposing a .set method that returns a new instance of the record in a type-safe way.

But since it's based on Immutable.Maps under the hood you may or may not be interested in it depending on your use case.

@danielfigueiredo
Copy link
Contributor

danielfigueiredo commented Aug 30, 2016

@SethDavenport is right, spread objects can get tricky because you could combine an infinite number of types, my guess it would return an intersection of all. As long as you don't have any in the pack it would validate known properties.
If you are not into Immutable so that you could benefit from Typed Record, maybe you can have another simple solution, it would require you to write more types but maybe that is what you want. You could combine two or more different types in typescript using Object.assign and a utility method. This method would take in type A, and type B and return both types merged.

describe('test', () => {
  it('intersection', () => {

    function assign<A, B>(a: A, b: B): A & B {
      return Object.assign({}, a, b);
    }

    interface MyA {
      propA: string;
    }

    interface MyB {
      propB: number;
    }

    const aB = assign<MyA, MyB>({propA: 'a'}, {propB: 1});
    expect(aB.propA).to.not.null;
    expect(aB.propB).to.not.null;

  });
});

Not sure if that is what you want =] Combining an unknown number of types I'm not sure if it is possible.

@danielfigueiredo
Copy link
Contributor

Also found this that might be interesting
microsoft/TypeScript#5453
and maybe this depending on what you want to do :
microsoft/TypeScript#6502

@SethDavenport
Copy link
Member

Thanks for the links @danielfigueiredo, seems like lots of people are interested in this topic.

By default, the typings for Object.assign<T, U, V>(...) return an intersection type (T & U & V) which is not that useful in this scenario IMO.

What would be nice is a version of assign that treats the type of the original object as a the 'master' type, and the types of subsequent arguments as subsets of that master type.

Fortunately it looks like TypeScript 1.8 introduced features that make this possible: https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#type-parameters-as-constraints.

Unfortunately the example given there is mutating, whereas we want to construct a new object of the master type.

@SethDavenport
Copy link
Member

SethDavenport commented Aug 31, 2016

However we can use Object.assign internally to avoid mutation and wrap it in a strongly typed interface of our choosing. I'm thinking something like this:

function tassign<T extends U, U>(target: T, source: U): T {
  return Object.assign({}, target, source);
}

let x = { a: 1, b: 2, c: 3, d: 4 };
tassign(x, { b: 10, d: 20 }); // OK

// Error: Property 'e' is missing in type
// '{ a: number; b: number; c: number; d: number; }
tassign(x, { e: 0 });

I'm only having issues making it variadic, but that might not be strictly needed in this case.

@SethDavenport
Copy link
Member

Putting it all together into a typesafe reducer would look like this:

import { Reducer, Action } from 'redux';

const INITIAL_STATE: ITripState = {
  loading: false,
  trips: null
};

export interface ITripState {
  loading: boolean;
  trips: string[];
}

export const tripReducer: Reducer<ITripState> = (
  state: ITripState = INITIAL_STATE,
  action: Action) => {

  switch(action.type) {
    case TRIP_ACTIONS.QUERY_TRIPS_ASYNC:
      if (state.loading) return state;
      return tassign(state, {
        loading: true,
        trips: null,
        foo: 'sdas'
      });
  }
}

@SethDavenport
Copy link
Member

@e-schultz would it be worth packaging the tassign implementation above with ng2-redux?

@sobvan
Copy link
Author

sobvan commented Aug 31, 2016

@SethDavenport, @danielfigueiredo thanks for your deep discussion on this. tassign looks really great. Also thanks for the rewrite, I will definitely give it a try.
I also think, that this would be a big plus for ng2-redux!
The most interesting part of your answer @SethDavenport, is that my code looks almost identincal to what you have written. This means to me that providing some code generation for the reducers and actions would make a lot of sense, indeed. And this takes us back to the original question :)

@danielfigueiredo
Copy link
Contributor

danielfigueiredo commented Aug 31, 2016

@SethDavenport thinking about this reducer and state use case you're absolutely right.
Because we know the shape state it is possible to restrict sources to something that the state shape knows, or in other words would pass in a T instanceof U test, which is great
You can count my votes for tassign inside ng2-redux!
I would slightly change it to the following:

function tassign<T extends U, U>(target: T, ...source: U[]): T {
  return Object.assign({}, target, ...source);
}

let x = { a: 1, b: 2, c: 3, d: 4 };
tassign(x, { b: 10, d: 20 });

@SethDavenport
Copy link
Member

@danielfigueiredo yeah unfortunately the variadic formulation doesn't work (I tried that earlier). Just try doing this:

let x = { a: 1, b: 2, c: 3, d: 4 };
tassign(x, { b: 10 }, {d: 20 });

The issue is that here we have 3 types. In general you'd need N types for this to work.

@danielfigueiredo
Copy link
Contributor

@SethDavenport gahh you're right, and makes sense now
Look forward to typescript add support to these nice features

@SethDavenport
Copy link
Member

SethDavenport commented Sep 1, 2016

@istvanszoboszlai to be honest I don't have much experience with code generation. I prefer to type it out myself, helps me remember what I did :)

What tools have you been using?

@SethDavenport
Copy link
Member

Closing this issue for now; code generation isn't something we have much experience with. If someone wants to implement code generation as an external module, I'm happy to advise. If such a module could also work for ngrx/store that would be even better :)

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

No branches or pull requests

3 participants