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

Api name #24

Closed
mweststrate opened this Issue Jan 3, 2018 · 17 comments

Comments

Projects
None yet
7 participants
@mweststrate
Owner

mweststrate commented Jan 3, 2018

I am not too excited about immer as function name as it is undescriptive. What should the idiomatic name be?

Transform? Update? withMutations? Make?

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Jan 3, 2018

Owner

Produce? Next? Mutate? Modify?

Owner

mweststrate commented Jan 3, 2018

Produce? Next? Mutate? Modify?

@Gregjarvez

This comment has been minimized.

Show comment
Hide comment
@Gregjarvez

Gregjarvez Jan 3, 2018

Contributor

I was thinking about it too. Since it was designed primarily with redux in mind. i think the idiomatic name should contain redux-
maybe redux-musc, musc , musk. This package abstracts (musks) aways immutability lol

Contributor

Gregjarvez commented Jan 3, 2018

I was thinking about it too. Since it was designed primarily with redux in mind. i think the idiomatic name should contain redux-
maybe redux-musc, musc , musk. This package abstracts (musks) aways immutability lol

@AriaFallah

This comment has been minimized.

Show comment
Hide comment
@AriaFallah

AriaFallah Jan 4, 2018

@Gregjarvez this isn't coupled to redux though. It's for anything that you need to update immutably. For example, I've used it in just plain setState.

Regarding the actual name, I think "Create the next immutable state" from the README really captures the essence of what immer does. Thus, if verbosity wasn't an issue, then createNextStateImmutably would perfectly describe it. However, I do think that's a bit wordy so I maybe something like create, createNext, or nextState would work.

@mweststrate Of your suggestions, I like transform and make the most. Not a fan of update, mutate, or modify because those clash with the immutable nature of the library.

AriaFallah commented Jan 4, 2018

@Gregjarvez this isn't coupled to redux though. It's for anything that you need to update immutably. For example, I've used it in just plain setState.

Regarding the actual name, I think "Create the next immutable state" from the README really captures the essence of what immer does. Thus, if verbosity wasn't an issue, then createNextStateImmutably would perfectly describe it. However, I do think that's a bit wordy so I maybe something like create, createNext, or nextState would work.

@mweststrate Of your suggestions, I like transform and make the most. Not a fan of update, mutate, or modify because those clash with the immutable nature of the library.

@benbraou

This comment has been minimized.

Show comment
Hide comment
@benbraou

benbraou Jan 4, 2018

Collaborator

I likewithMutations because it is familiar (immutable.js)
next and produce also convey the puropose of the library

Collaborator

benbraou commented Jan 4, 2018

I likewithMutations because it is familiar (immutable.js)
next and produce also convey the puropose of the library

@hex13

This comment has been minimized.

Show comment
Hide comment
@hex13

hex13 Jan 4, 2018

I named similar function transform in my transmutable library.
https://github.com/hex13/enter-ghost/tree/master/packages/transmutable
I think it's kind of have sense - we're transforming the state by giving recipe of "how to transform"

On the other side redux.* would be horrible name - what if somebody would like to use this library out of Redux context? Coupling is bad, even in naming.

next would look good in context of Redux reducers, but it would look weird in anywhere else.

I think update, mutate, or modify are also good.

I came up also with run name (something like controlled running - here a state, run some code in context of this state and return result).

Or commit (because we "commit" mutations - like in Vuex).

two or more words would be probably too verbose for helper library.

hex13 commented Jan 4, 2018

I named similar function transform in my transmutable library.
https://github.com/hex13/enter-ghost/tree/master/packages/transmutable
I think it's kind of have sense - we're transforming the state by giving recipe of "how to transform"

On the other side redux.* would be horrible name - what if somebody would like to use this library out of Redux context? Coupling is bad, even in naming.

next would look good in context of Redux reducers, but it would look weird in anywhere else.

I think update, mutate, or modify are also good.

I came up also with run name (something like controlled running - here a state, run some code in context of this state and return result).

Or commit (because we "commit" mutations - like in Vuex).

two or more words would be probably too verbose for helper library.

@alexkrolick

This comment has been minimized.

Show comment
Hide comment
@alexkrolick

alexkrolick Jan 4, 2018

Some ideas:

  • finalize
  • finalizeDraft
  • applyDraftChanges
  • applyDiffFromDraft
  • applyDiff
  • newStateFromDraft

I like having draft in there because it makes it clear that the function is doing work with the return value of the 2nd argument.

alexkrolick commented Jan 4, 2018

Some ideas:

  • finalize
  • finalizeDraft
  • applyDraftChanges
  • applyDiffFromDraft
  • applyDiff
  • newStateFromDraft

I like having draft in there because it makes it clear that the function is doing work with the return value of the 2nd argument.

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Jan 4, 2018

Owner

Or just

import ✏ from "immer"

function insertItem(array, action) {
    return ✏(array, draft => {
        draft.splice(action.index, 0, action.item)
    })
}

:)

edit: that doesn't compile. Too bad, would be sooo cool.

Owner

mweststrate commented Jan 4, 2018

Or just

import ✏ from "immer"

function insertItem(array, action) {
    return ✏(array, draft => {
        draft.splice(action.index, 0, action.item)
    })
}

:)

edit: that doesn't compile. Too bad, would be sooo cool.

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Jan 4, 2018

Owner

I'm thinking about transform(state, recipe: draft => {}), it is short, makes quite clear that it produces a next state from the current one, and makes it nicely explainable:

"transform the current state to produce the next state according to this recipe that expresses how a draft state should be changed"

Owner

mweststrate commented Jan 4, 2018

I'm thinking about transform(state, recipe: draft => {}), it is short, makes quite clear that it produces a next state from the current one, and makes it nicely explainable:

"transform the current state to produce the next state according to this recipe that expresses how a draft state should be changed"

@alexkrolick

This comment has been minimized.

Show comment
Hide comment
@alexkrolick

alexkrolick Jan 4, 2018

"Transform" implies mutations, to me.

nextState(currentState, transform)

🤔

alexkrolick commented Jan 4, 2018

"Transform" implies mutations, to me.

nextState(currentState, transform)

🤔

@Gregjarvez

This comment has been minimized.

Show comment
Hide comment
@Gregjarvez

Gregjarvez Jan 4, 2018

Contributor

if the draft eventually becomes the next state

function insertItem(array, action) {
    return draft(array, nextState=> {
        nextState.splice(action.index, 0, action.item)
    })
}

:)

Contributor

Gregjarvez commented Jan 4, 2018

if the draft eventually becomes the next state

function insertItem(array, action) {
    return draft(array, nextState=> {
        nextState.splice(action.index, 0, action.item)
    })
}

:)

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Jan 4, 2018

Owner

I think @alexkrolick brings up a good point, transform might imply modification existing data for many. produce probably makes it clearer something new is created

function insertItem(array, action) {
    return produce(array, draft => {
        draft.splice(action.index, 0, action.item)
    })
}
Owner

mweststrate commented Jan 4, 2018

I think @alexkrolick brings up a good point, transform might imply modification existing data for many. produce probably makes it clearer something new is created

function insertItem(array, action) {
    return produce(array, draft => {
        draft.splice(action.index, 0, action.item)
    })
}
@AjaxSolutions

This comment has been minimized.

Show comment
Hide comment
@AjaxSolutions

AjaxSolutions Jan 4, 2018

Definitely not immer.

The Urban Dictionary defines immer as someone who IMs a lot.

AjaxSolutions commented Jan 4, 2018

Definitely not immer.

The Urban Dictionary defines immer as someone who IMs a lot.

@hex13

This comment has been minimized.

Show comment
Hide comment
@hex13

hex13 Jan 4, 2018

two ideas:

  1. swap parameters (recipe, state) instead of other way around. It could be counterintuitive but it could help with partial application and reusing transformations.
  2. allow for passing additional parameters to recipe function
const immer = require('immer').default;
const {createStore} = require('redux');

// proposal: if immer api look this way (fn, state, ...args):
const transform = (fn, state, ...args) => immer(state, draft => fn(draft, ...args));

//-----------------------------------------------
const initialState = {
    animals: ['cat', 'dog']
};

const addAnimal = (state, action) => {
    state.animals.push(action.animal);
};

const reducer = transform.bind(null, (state,  action) => {
    switch (action.type) {
        case 'add':
            addAnimal(state, action);
            break;
    }
});

const store = createStore(reducer, initialState);
store.subscribe(() => {
    console.log("STATE", store.getState())
});

store.dispatch({type: 'add', animal: 'monkey'});
store.dispatch({type: 'add', animal: 'chicken'});

though this is one liner but having such things in library out-of-the-box could be nice. Or it could use curry instead of partial application

(related: integration with Redux #31
#26 )

(but this is not only Redux related. This pattern could be helpful in many things, where we would like to store some recipe for transform for later use).

hex13 commented Jan 4, 2018

two ideas:

  1. swap parameters (recipe, state) instead of other way around. It could be counterintuitive but it could help with partial application and reusing transformations.
  2. allow for passing additional parameters to recipe function
const immer = require('immer').default;
const {createStore} = require('redux');

// proposal: if immer api look this way (fn, state, ...args):
const transform = (fn, state, ...args) => immer(state, draft => fn(draft, ...args));

//-----------------------------------------------
const initialState = {
    animals: ['cat', 'dog']
};

const addAnimal = (state, action) => {
    state.animals.push(action.animal);
};

const reducer = transform.bind(null, (state,  action) => {
    switch (action.type) {
        case 'add':
            addAnimal(state, action);
            break;
    }
});

const store = createStore(reducer, initialState);
store.subscribe(() => {
    console.log("STATE", store.getState())
});

store.dispatch({type: 'add', animal: 'monkey'});
store.dispatch({type: 'add', animal: 'chicken'});

though this is one liner but having such things in library out-of-the-box could be nice. Or it could use curry instead of partial application

(related: integration with Redux #31
#26 )

(but this is not only Redux related. This pattern could be helpful in many things, where we would like to store some recipe for transform for later use).

@alexkrolick

This comment has been minimized.

Show comment
Hide comment
@alexkrolick

alexkrolick Jan 4, 2018

@hex13 file a new issue for that? A separate thread would be nice.

alexkrolick commented Jan 4, 2018

@hex13 file a new issue for that? A separate thread would be nice.

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Jan 4, 2018

Owner

@hex13 indeed, deserves separate issue :) also see #31, quite related

Owner

mweststrate commented Jan 4, 2018

@hex13 indeed, deserves separate issue :) also see #31, quite related

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Jan 9, 2018

Owner

produce it will be. It allows for the concept of producers which nicely rhymes with reducers :). Will soon update. Or if someone beats me to it that is fine as well ;-). (make sure to claim the issue)

Owner

mweststrate commented Jan 9, 2018

produce it will be. It allows for the concept of producers which nicely rhymes with reducers :). Will soon update. Or if someone beats me to it that is fine as well ;-). (make sure to claim the issue)

@mweststrate mweststrate added this to the 1.0 milestone Jan 9, 2018

@mweststrate mweststrate self-assigned this Jan 9, 2018

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate
Owner

mweststrate commented Jan 9, 2018

Done

@mweststrate mweststrate closed this Jan 9, 2018

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