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

Remove type constraint for Output type of epics #560

Closed
DevWurm opened this issue Aug 20, 2018 · 8 comments
Closed

Remove type constraint for Output type of epics #560

DevWurm opened this issue Aug 20, 2018 · 8 comments

Comments

@DevWurm
Copy link

DevWurm commented Aug 20, 2018

What is the current behavior?
Currently the Action Output type is constrained to extend the Action Input type of an Epic.

What is the expected behavior?
The Output type should not be constrained, because there is no need to limit the Output actions as far as I see.

@DevWurm DevWurm mentioned this issue Aug 20, 2018
2 tasks
@evertbouw
Copy link
Member

See #446 for the logic behind the current typings

The reason the output extends the input is because technically speaking every action that is output is always recursively provided as the input, it's just usually filtered out by ofType. As a bit of a purist I prefer to have things as accurate as possible because while sometimes a pain it could potentially prevent mistakes. e.g. if you don't provide your output actions as inputs you could potentially make a mistake in your custom filtering logic, though I'll admit it's not likely to be an issue for most people in practice.

@Ailrun
Copy link
Contributor

Ailrun commented Aug 20, 2018

Input should be super type of output, since your result action (output) always goes back to your epic input. i.e., to use valid input type, your input type should include output type.

@jayphelps
Copy link
Member

@DevWurm does that make sense?

@DevWurm
Copy link
Author

DevWurm commented Sep 3, 2018

Sorry for the late answer! I understand the reason now and agree. The perfect solution would be to have a type, which described all possible Actions for the Input, but because we didn't find an easy solution for this, I agree with the way you typed it as a middle ground.
Thanks for your help @evertbouw @Ailrun @jayphelps!

@DevWurm DevWurm closed this as completed Sep 3, 2018
@AyanGhatak
Copy link

Hey guys,

As from index.d.ts#L36-L38

export declare interface Epic<Input extends Action = any, Output extends Input = Input, State = any, Dependencies = any> {

But for the following snippet,

export const myEpic: Epic<InputType, OutputType> = (action$) => action$.pipe(
  ofType('action1'),
  bufferTime(300),
  map((payload) => {type: 'action2', payload})
);

How do we expect to hold the constrain Output extends Input ? Isn't Output likely to be Input[] for buffer like operator ?

Any help is appreciated. cc: @evertbouw @Ailrun

@evertbouw
Copy link
Member

I think I don't understand your question. bufferTime will give you an array of values, so in your map operator payload is an InputType[].

InputType is a union of all of your actions and OutputType is a subset of those actions. If you define action2 to hold a bunch of action1s this constraint still holds.

@AyanGhatak
Copy link

Thanks Evert for the prompt reply. Ok this makes sense.

InputType is a union of all of your actions and OutputType is a subset of those actions

Using the union operator and then while returning using the as operator works for me. Thanks @liqiang372 for the offline help.

Code snippet:

export interface MyAction<T> {
  type: string; // 'action1' or 'action2'
  payload: T;
}

type InputType = MyAction<something>;
type OutputType = MyAction<something[]>;

export const myEpic: Epic<InputType | OutputType, OutputType> = (action$) => action$.pipe(
    ofType('action1'),
    bufferTime(bufferDuration),
    map((value: fetchPanelInputAction[]) => ({
      type: 'action2',
      payload: value.map(({ payload }) => payload),
    } as OutputType)),
  );

Incidentally its close to what Jay suggested here. 😄

@henrist
Copy link

henrist commented Jan 9, 2019

Having the output extending the input breaks dispatching thunks from an observable (thunks don't extend Action). As thunks should be handled by the thunk middleware before reaching redux-observable middleware, the thunk is not considered input too.

Reference: https://stackoverflow.com/a/48512442 (by @jayphelps)

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

6 participants