This repository has been archived by the owner. It is now read-only.

Support typing for command and executor `payload`, defaulted to `object` #152

Merged
merged 9 commits into from Jan 11, 2018

Conversation

Projects
None yet
3 participants
@agubler
Member

agubler commented Jan 4, 2018

Type: bug

The following has been addressed in the PR:

Description:

Switches the payload from rest params (...payload: any[]) type to a single mandatory type that extends object.

The type of payload can be defined by a second generic for both commands and createProcess.

const createCommandOne = createCommandFactory<any, { foo: string }>();
const createCommandTwo = createCommandFactory();
const commandOne = createCommandOne(({ get, path, payload }) => []);
const commandTwo = createCommandTwo(({ get, path, payload }) => []);
const processOne = createProcess([commandOne]);

createProcess<any, { bar: string }>([commandOne]); // compilation error
const processTwo = createProcess([commandTwo]);
const executorOne = processOne(store);
const executorTwo = processTwo(store);

executorOne({}); // compilation error
executorOne({ foo: 'bar' });
executorTwo({ foo: 'bar' });
executorTwo({});
executorTwo(1); // compilation error
executorTwo(''); // compilation error
executorTwo(); // compilation error

Resolves #146

@agubler agubler requested a review from matt-gadd Jan 4, 2018

@agubler

This comment has been minimized.

Member

agubler commented Jan 4, 2018

Needs changes to the README

agubler added some commits Jan 5, 2018

@agubler agubler requested a review from maier49 Jan 6, 2018

@agubler

This comment has been minimized.

Member

agubler commented Jan 8, 2018

@maier49 hey, do you think you'll have time for a review this week?

agubler added some commits Jan 8, 2018

README.md Outdated
];
return operations;
}
```
A `Command`, or the `CommandRequest` argument to it, can be provided with a generic that indicates the type of the state of the
store they are intended to target. This will provide type checking for all calls to `path` and `at`, ensuring that the operations will be targeting real properties of the store, and providing type inference for the return type of `get`. In order to avoid typing each `Command` explicitly, a `CommandFactory` can be created that will pass its generic type onto all commands it creates. Creating commands using a factory is essentially the same as creating them without one. It is simply a convenience to avoid repeating the same type for each command.
A `Command`, or the `CommandRequest` argument to it, can be provided with a generics that indicates the type of the state of the store they are intended to target and the payload that will be passed. This will provide type checking for all calls to `path` and `at` and usages of `payload`, ensuring that the operations will be targeting real properties of the store, and providing type inference for the return type of `get`.

This comment has been minimized.

@maier49

maier49 Jan 10, 2018

Contributor

Should be "with generics" instead of "with a generics"

README.md Outdated
@@ -283,19 +334,53 @@ The `undo` function will rollback all the operations that were performed by the
### Transforming Executor Arguments
An optional `transformer` can be passed to the `createExecutor` function that will be used to parse the arguments passed to the executor.
An optional `transformer` can be passed to a process that is be used to transform the `executor`s payload to the `command` payload type. The return type of the `transformer` must match the `command` `payload` type of the `process`. The argument type of the `executor` is inferred from transformers `payload` type.

This comment has been minimized.

@maier49

maier49 Jan 10, 2018

Contributor

"is be used" -> "is used"

export interface CommandFactory<T = any> {
(command: Command<T>): Command<T>;
export interface CommandFactory<T = any, P extends object = object> {
<R extends object = P>(command: Command<T, R>): Command<T, R>;

This comment has been minimized.

@maier49

maier49 Jan 10, 2018

Contributor

Looking through the docs, my first thought was that it would make sense to let the generic type for the payload be specific on the call to create the command as well as on the command factory itself. It appears you can, but the docs seem to show multiple factories being created instead of using this pattern. Is there any reason for that?

This comment has been minimized.

@agubler

agubler Jan 10, 2018

Member

Sure, I can add example for that.

@@ -92,15 +99,24 @@ export interface ProcessCallbackDecorator {
/**
* CreateProcess factory interface
*/
export interface CreateProcess {
<T>(commands: (Command<T>[] | Command<T>)[], callback?: ProcessCallback<T>): Process<T>;
export interface CreateProcess<T = any, P extends object = object> {

This comment has been minimized.

@maier49

maier49 Jan 10, 2018

Contributor

Would it be worthwhile to provide overrides for 1 to N commands where the payload type is the intersection of the payload types for each command?

agubler added some commits Jan 10, 2018

@agubler agubler merged commit 66c868e into dojo:master Jan 11, 2018

4 checks passed

codecov/patch 100% of diff hit (target 99.15%)
Details
codecov/project 99.15% (+<.01%) compared to 543ebdf
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@dylans dylans added this to the beta.5 milestone Jan 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.