meiosis-setup with typescript: some suggestions

[erikvullings]

@foxdonut It is really impressive what you've achieved with the TypeScript support, so I have only a few remarks/suggestions/requests.

1. Can we reduce the number of import lines?

Currently:

import Stream from 'mithril/stream';
import { toStream } from 'meiosis-setup/common';
import { App, Service, setup } from 'meiosis-setup/mergerino';
import { merge } from '../utils/mergerino';

I would prefer:

import Stream from 'mithril/stream';
import { toStream } from 'meiosis-setup/common';
import { App, Service, setup, merge } from 'meiosis-setup/mergerino';

As we are already using mergerino, it makes little sense (considering it is quite stable) to import it separately.

2. What does getCell do? It has no documentation.

   export const { states, getCell } = setup({ stream: toStream(Stream), merge, app });

3. In your example, you still use:

   const { states, update, actions } = meiosis({ stream: simpleStream, merge, app });

   However, in your code, I do not see the actions being exported, and instead, you export states and getCell? And in the mergerino version, getCell also has a nest function. What is that used for?

4. Considering we get setup from meiosis-setup/mergerino, it would make sense that the merge does not have to be supplied separately, i.e. why can't we use:

   export const { states, getCell } = setup({ stream: toStream(Stream), app });

5. Why is toStream first exported, just to convert Stream to StreamLib? Can't this be done internally, so instead, pass

   export const { states, getCell } = setup({ stream: Stream, app });

   Where Stream is of type mithril/stream or flyd.

6. Alternatively, as I see that you have implemented a simpleStream yourself, why not leave it out, so it becomes:

   
   import { App, Service, setup } from 'meiosis-setup/mergerino';

export const { states, getCell } = setup({ app });

So the `simpleStream` will be the default value for `stream`, the same with `merge` and `mergerino`.

7. When setting up the `app`, as in:
```ts
const app: App<IAppModel, IActions> = {
  initial: Object.assign({}, appStateMgmt.initial) as IAppModel,
  Actions: context => Object.assign({}, appStateMgmt.actions(context)) as IActions,
  /** Services update the state */
  services: [] as Array<Service<IAppModel>>,
};

Why do you write services using lower-case letters, versus Actions and Effects? I prefer the former, but perhaps there is a specific reason for that.

8. Personal style thingy: In memory-trainer, file app-state.ts, you have the following

   actions: context => { // ...

   Whereas I prefer, so I don't have to write context.update(...) everywhere:

   actions: ({ update, getState }) => { // ...

9. When creating actions using arrow functions, you cannot call another action. Using function, you can, by using this. However, Typescript coding practices do not really like the usage of this. Isn't it possible to also provide the actions, like the state and update function, as a parameter (in the context as defined before)? So you can always get to another action. I.e. the above example would become:

   actions: ({ update, getState, actions }) => { // ...

[foxdonut]

@erikvullings thank you for your feedback and suggestions! I will reply to each item below:

1. Can we reduce the number of import lines?

Currently:

import Stream from 'mithril/stream';
import { toStream } from 'meiosis-setup/common';
import { App, Service, setup } from 'meiosis-setup/mergerino';
import { merge } from '../utils/mergerino';

I would prefer:

import Stream from 'mithril/stream';
import { toStream } from 'meiosis-setup/common';
import { App, Service, setup, merge } from 'meiosis-setup/mergerino';

As we are already using mergerino, it makes little sense (considering it is quite stable) to import it separately.

That is an excellent idea. Will do.

2. What does getCell do? It has no documentation.

export const { states, getCell } = setup({ stream: toStream(Stream), merge, app });

Indeed this is all work-in-progress, and some things still may change, so documentation is not yet written.

3. In your example, you still use:

const { states, update, actions } = meiosis({ stream: simpleStream, merge, app });

However, in your code, I do not see the actions being exported, and instead, you export states and getCell? And in the mergerino version, getCell also has a nest function. What is that used for?

Same as previous item, still work-in-progress and not yet complete.

4. Considering we get setup from meiosis-setup/mergerino, it would make sense that the merge does not have to be supplied separately, i.e. why can't we use:

export const { states, getCell } = setup({ stream: toStream(Stream), app });

Great point, I will combine with item 1 to simplify the mergerino setup.

5. Why is toStream first exported, just to convert Stream to StreamLib? Can't this be done internally, so instead, pass

export const { states, getCell } = setup({ stream: Stream, app });

Where Stream is of type mithril/stream or flyd.

This is because TypeScript gives me a hard time with the mithril/stream and flyd types. But you're absolutely right, this should not be exposed to the user. I will find a way to resolve internally within the library so that the user does not need to be bothered by this.

6. Alternatively, as I see that you have implemented a simpleStream yourself, why not leave it out, so it becomes:

import { App, Service, setup } from 'meiosis-setup/mergerino';

export const { states, getCell } = setup({ app });

So the simpleStream will be the default value for stream, the same with merge and mergerino.

I wanted the user to be able to choose which stream implementation they want to use, but you're right, simpleStream should be the default. The user can still have the option to use flyd or mithril-stream, but at the same time they can use meiosis-setup with simpleStream out of the box without having to specify a stream implementation.

7. When setting up the app, as in:

const app: App<IAppModel, IActions> = {
  initial: Object.assign({}, appStateMgmt.initial) as IAppModel,
  Actions: context => Object.assign({}, appStateMgmt.actions(context)) as IActions,
  /** Services update the state */
  services: [] as Array<Service<IAppModel>>,
};

Why do you write services using lower-case letters, versus Actions and Effects? I prefer the former, but perhaps there is a specific reason for that.

The reason is that Actions and Effects are constructors, in other words functions that return actions and effects, whereas services is directly an array of services. But, this will most likely change as I am still working on the API design.

8. Personal style thingy: In memory-trainer, file app-state.ts, you have the following

actions: context => { // ...

Whereas I prefer, so I don't have to write context.update(...) everywhere:

actions: ({ update, getState }) => { // ...

Agreed, you can definitely use the style that you prefer.

9. When creating actions using arrow functions, you cannot call another action. Using function, you can, by using this. However, Typescript coding practices do not really like the usage of this. Isn't it possible to also provide the actions, like the state and update function, as a parameter (in the context as defined before)? So you can always get to another action. I.e. the above example would become:

actions: ({ update, getState, actions }) => { // ...

I am debating making actions be just regular helper functions outside of meiosis-setup. In my experimentation this makes it easier and cleaner to organise actions as you wish, and does not force you to "assemble" all your actions in the setup. (You can still do that if you wish, but you are not obligated.)

[foxdonut]

@erikvullings I have updated meiosis-setup as well as my branch of memory-trainer with your suggested improvements 👍

[erikvullings]

@foxdonut thanks for taking a look at my suggestions. Will report back after checking out the new version!

[erikvullings]

@foxdonut Sorry for the wait, and thanks for the update and the many improvements you have added!

I'm reporting back on beta 10 as used in memory-trainer.

1. In meiosis.ts, I see the following code (line 27).

const { states, getCell } = setup({ app });

const actions: IActions = createActions({ getState: states, update: getCell().update });

export const getMeiosisCell = () => ({
  ...getCell(),
  actions,
});

Changing states to getState is done twice: if setup would return getState, that would be simpler. Alternatively, if createActions would accept a states property, that would work too. I have a slight preference for the latter approach, since getState implies a get method, whereas it is still a stream of states (so I could add a scan or map function to it too).

Similarly, the getCell()

So the previous code would become:

const { states, getCell } = setup({ app });

const actions: IActions = createActions({ states, update: getCell().update });

2. setup returns a getCell function. I like the name meiosis, but I do not like the name getCell so much, as I do not understand what it does (I like function names that indicate what they do). Also, there seems to be some overlap in that setup returns states and getCell, and I assume that states() === getCell().state. If that is correct, you return the same information twice. So ignoring nest, you could have something like:

const { states, update } = setup({ app });

const actions: IActions = createActions({ states, update });

export const getMeiosisCell = () => ({
  state: states(),
  update,
  actions,
});

3. Similar as above, getMeiosisCell does not resound well about what it does. I've thought about getAppMgmtTools or getAppUtensils, but anyways, that is in my own part of the code.
4. About your remark on keeping the actions outside meiosis setup... Indeed, in a more complex application that uses meiosis, it can be a bit of a pain to "assemble" the actions into the app-state for every set of actions that you add. On the other hand, a new set of actions also implies an addition to the app-state, so if I have to edit the app-state anyways, adding the new actions is not that much more difficult. Or would it be possible to add a new bit of state plus actions to an already existing application state, e.g. in case you develop a dashboard application, and a new widget has some state, actions (and services/effects?), is it possible to add the widget's state and actions to the app state without editing it manually (while retaining Typescript's autocomplete and type checking)?

[foxdonut]

@erikvullings thank you for having a look and for your feedback, much appreciated!

Indeed for states vs getState this is done in the person's application, not in meiosis-setup, so you would choose the name that you prefer.

Normally for what I envision you wouldn't have createActions, instead you would pass cell to actions which will enable you to use nesting -- more on this later, I am still working on it.

I agree that there is overlap between states and getCell, and I am debating and experimenting with removing that overlap.

Since you are looking at this before I have documented the project, indeed getCell doesn't convey its purpose as well as it should. Hopefully that will be clearer when the project is documented. The idea is to have one object that you can pass to your view, instead of always having to repeat something like { state, update, actions }. I gave the name "cell" to that object because it goes well with "meiosis" (given that in science, meiosis has to do with cells.)

Finally I am also working on what you said, adding a new bit of state to an existing application state. This also has to do with nesting which I mentioned earlier. I am still working on being able to add a new bit of state, together with actions and services. I am eliminating effects, services will be able to do both sync and async updates.

I will let you know as these new things become available, if you are then able to have a look I will appreciate your feedback, thanks again!

[erikvullings]

Thanks for closing this issue. I should have done it already, since I'm already using your improvements in the latest version. Great work, and I really love using it!

On Sat, 22 Oct 2022, 00:37 Fred Daoud, @.***> wrote:

Closed #172 https://github.com/foxdonut/meiosis/issues/172 as completed.

— Reply to this email directly, view it on GitHub https://github.com/foxdonut/meiosis/issues/172#event-7643834922, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAX6YO24YFAMIFEEOKO44SDWEMEKZANCNFSM5PPECC5Q . You are receiving this because you were mentioned.Message ID: @.***>

[foxdonut]

@erikvullings heartfelt thanks for all your help and for your kind words, very much appreciated!