Extract defaultProps detection #1942
Conversation
alexzherdev
force-pushed
the
default-props-detection
branch
from
48e4be0 to
b588947
Compare
lib/util/Components.js
Outdated
| const allKeys = new Set(Object.keys(detectionInstructions).concat(Object.keys(propTypesInstructions))); | ||
| const defaultPropsInstructions = defaultProps(context, components, utils); | ||
| const allKeys = new Set(Object.keys(detectionInstructions).concat(Object.keys(propTypesInstructions)) | ||
| .concat(Object.keys(defaultPropsInstructions))); |
There was a problem hiding this comment.
this doesn't need to be two concat calls, it can be arr1.concat(arr2, arr3)
lib/util/defaultProps.js
Outdated
| } | ||
|
|
||
| const defaults = component.defaultProps || {}; | ||
| const newDefaultProps = defaultProps.reduce((acc, prop) => { |
There was a problem hiding this comment.
let's use the object.fromentries package here.
alexzherdev
force-pushed
the
default-props-detection
branch
from
b588947 to
d50df15
Compare
alexzherdev
force-pushed
the
default-props-detection
branch
2 times, most recently
from
3f41753 to
ce0b2d6
Compare
alexzherdev
force-pushed
the
default-props-detection
branch
from
ce0b2d6 to
b81e19a
Compare
|
@ljharb FYI I have a final refactoring ready but blocked by this, which is to extract |
|
|
||
| if (!isPropType && !isDefaultProp) { | ||
| if (!isPropType) { |
There was a problem hiding this comment.
i'm a bit confused why the previous propTypes refactor didn't remove isPropType, but this defaultProps refactor is able to remove isDefaultProp. can you elaborate?
There was a problem hiding this comment.
Sure.
The first extraction was based on two other rules that had the most comprehensive detection functionality. However, isRequired detection, which is necessary for the two rules in this current PR, was not part of it. I could have added it there, but that would have blown up an already big PR.
But it is coming in the follow-up PR I mentioned above. I just didn’t want to combine it with this one, again, to keep them focused and manageable in size.
See alexzherdev@572e332 and alexzherdev@8add5de for an exclusive peek 😄
There was a problem hiding this comment.
ah ok, gotcha. so all the isPropType stuff will be able to be removed in a followup.
|
Will merge later today if no objections. |
|
Any concerns here? |
ghost
mentioned this pull request
No description provided.