codeforboston / cliff-effects

Cliff effects guidance prototype (archived)
https://codeforboston.github.io/cliff-effects/#/
MIT License
30 stars 64 forks source link

Refactor `sharedProps` prop in src/forms/CurrentExpenses.js #986

Open turnerhayes opened 5 years ago

turnerhayes commented 5 years ago

965 uses a prop called sharedProps for several components; this should be separated out into the specific props each component needs.

knod commented 5 years ago

Could you add an example of how it would look where it's separated?

turnerhayes commented 5 years ago

For example, the <Under13> component takes a sharedProps parameter. The format of that appears to be (looking where it's used):

{
    timeState:         current,
    type:              type,
    time:              time,
    updateClientValue: updateClientValue,
}

So instead of having the Under13 component be declared as

const Under13 = function ({ snippets, type, sharedProps, current, updateClientValue }) {
...
}

it could be

const Under13 = function ({ snippets, type, timeState, time, current, updateClientValue }) {
...
}

The code that uses <Under13> could do

 <Under13
          snippets          = { snippets }
          type              = { type }
          current           = { current }
          updateClientValue = { updateClientValue }
          { ...sharedProps } />

The advantage of this approach is that it's clearer what Under13 expects as its props, rather than having to inspect the code to see what ends up getting passed to it. Does that make sense?

knod commented 5 years ago

That's strange that updateClientValue and type are redundant in the shared props. I'll definitely think about it and other folks can as well, and look at how all those properties are being used in the code. I can see discussing some pros of a sharedProps object if it's used well, but this may or may not be the place for it.