Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

path-reducer #329

Closed
miqmago opened this issue May 19, 2016 · 5 comments
Closed

path-reducer #329

miqmago opened this issue May 19, 2016 · 5 comments

Comments

@miqmago
Copy link

miqmago commented May 19, 2016

I've developed this path-reducer which makes easy to save the form values inside redux state.

Please, let me know your opinion. Also some examples on how to implement it with tcomb-form and tcomb-form-native are coming in the future.

@gcanti
Copy link
Owner

gcanti commented May 19, 2016

Hi @miqmago,

Personally I like to decouple actions from state changes. Actions should just describe what happened, not what to do. So instead of:

const action = {
    type: 'updateElementInObject',
    meta: ['bar', 'boo'],
    payload: {
        boo: 1
    }
};

I'd choose something like:

const action = {
    type: 'update_requested',
    payload: {
        boo: 1
    }
};

This comment of @slorber (as lot of other comments of the same author) explains perfectly my thoughts: reduxjs/redux#1528 (comment)

@miqmago
Copy link
Author

miqmago commented May 20, 2016

Thanks. In that case, maybe there is a better option for this problem. I'm sorry for extending so much, but the problem is not easy and I very much appreciate your help:

I've got an structure with about +150 nested types (t.struct(t.struct(t.struct(...)))). In fact this is structured with import small files, each one defining a type and the options for the form presentation:

PartiesType.js

import t from 'tcomb-form-native';
import { getBusinessType, getBusinessTypeOptions } from './BusinessType';


export function getPartiesType(options) {
    const PartiesTypeBase = {
        SellerParty: getBusinessType(options),
        BuyerParty: getBusinessType(options),
    };

    return t.struct(PartiesTypeBase);
}

export function getPartiesTypeOptions(options) {
    return { fields: {} };
}

BusinessType.js

import t from 'tcomb-form-native';
import {
    getAdministrativeCentresType,
    getAdministrativeCentresTypeOptions,
    getIndividualType,
    getIndividualTypeOptions,
    getLegalEntityType,
    getLegalEntityTypeOptions,
    getPartyIdentificationType,
    getPartyIdentificationTypeOptions,
    getTaxIdentificationType,
    getTaxIdentificationTypeOptions,
} from '../reused-simple-enums';


export function getBusinessType(options) {
    const { personTypeCode = 'I' } = options;
    const BusinessTypeBase = {
        TaxIdentification: getTaxIdentificationType(options),
        PartyIdentification: getPartyIdentificationType(options),
        AdministrativeCentres: getAdministrativeCentresType(options),
        Individual: {},
        LegalEntity: {},
    };

    if (personTypeCode === 'J') {
        BusinessTypeBase.LegalEntity = getLegalEntityType(options);
        delete BusinessTypeBase.Individual;
    } else { // 'I' per defecte
        BusinessTypeBase.Individual = getIndividualType(options);
        delete BusinessTypeBase.LegalEntity;
    }
    return t.struct(BusinessTypeBase);
}

export function getBusinessTypeOptions(options) {
    return {
        auto: 'placeholders',
        fields: {
            TaxIdentification: getTaxIdentificationTypeOptions(options),
            PartyIdentification: getPartyIdentificationTypeOptions(options),
            AdministrativeCentres: getAdministrativeCentresTypeOptions(options),
            Individual: getIndividualTypeOptions(options),
            LegalEntity: getLegalEntityTypeOptions(options),
        },
    };
}

And so on. This structure makes easy to create the corresponding t.form.Form to ask the whole tree or part of it.

Personally I like to decouple actions from state changes

Does that mean that every change in a tcomb-form should be mapped to an specific action for the specific part of the tree that is being updated? I.e:

action = {
    type: 'update/Parties/SellerParty/TaxIdentification/PersonTypeCode',
    payload: {
        PersonTypeCode: 'J'
    }
};

Anyway this is a different way to let the action know what it has to be done. The difference is that it's difficult to operate with type, comparing with operate directly with an array in meta:

action = {
    type: 'updateEInvoice',
    meta: ['Parties', 'SellerParty', 'TaxIdentification', 'PersontTypeCode'],
    payload: {
        PersonTypeCode: 'J'
    }

Ths action is very easy to construct with the onChange method in t.form.Form, which already comes with the updated path.

Also does it mean that the whole structure should be duplicated with reducer functions, mapping the state? I personally think that it makes hard to maintain: every change made in tcomb type should be changed also in the state reducers.... Doesn't it make sense to move the knowledge of the tree to the action via meta path to the updated part of the tree?

This was the motivation when writting path-reducer: to not duplicate the tcomb structure with reducer's and actions structure...

@gcanti
Copy link
Owner

gcanti commented May 20, 2016

I see, makes sense. So it's a matter of taste I guess. I just prefer:

const action = {
    type: 'form_changed',
    payload: {
      what: ['bar', 'boo']
      value: 1
    }
}

but the result will be the same as yours.

Maybe I just can't see the use case and it's a dumb question, but out of curiosity, why are you interesting in such granular updates rather than storing the whole form value?

const action = {
    type: 'form_changed',
    payload: {
      ...whole form value here...
    }
}

Also, values coming from the onChange handler are not validated.

@miqmago
Copy link
Author

miqmago commented May 20, 2016

I like your proposal, I will try to update the plugin. It makes sense to keep all data in payload and maybe it's better for action validation purposes.

I'm interested in granular updates as the tcomb-native-form gives the exact path which is being updated, so I found this was the only option. Regarding the validation problem, what I'm trying to do is creating a new object with the whole data and validate it at the end.

The problem here is that due it is a big form, I had to split it in small forms that are being filled each one in a tab screen. The whole form is not submitted until all the screens are filled in. I still haven't dedicated enought time to think deep about the validation problem...

@gcanti
Copy link
Owner

gcanti commented Jul 3, 2016

Closing for inactivity

@gcanti gcanti closed this as completed Jul 3, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants