cliff-effects icon indicating copy to clipboard operation
cliff-effects copied to clipboard

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

Open turnerhayes opened this issue 7 years ago • 3 comments

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

turnerhayes avatar Nov 11 '18 01:11 turnerhayes

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

knod avatar Nov 11 '18 01:11 knod

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?

turnerhayes avatar Nov 12 '18 01:11 turnerhayes

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.

knod avatar Nov 12 '18 01:11 knod