[no-object-literal-type-assertion] Allo 8000 w for function properties option ("defaultProps") #519
Comments
|
I don't agree with this enhancement for a few reasons:
const defaultProps: Pick<Props, 'myProp'> = {
myProp: 'hello world'
}
Comp.defaultProps = defaultProps;function Comp(props: Props) {
////////
}
namespace Comp {
export const defaultProps: Partial<Props> = {
myProp: 'hello world'
};
}That being said, I won't bl 8000 ock on it. |
I don't think it's "very, very specific", given how popular React is, and that with the release of hooks functional components are being used more and more.
Yup that's a big change alright. I'd had hoped that wasn't required, but now that I look into I see the matcher is smaller than I expected (as it's just looking for
The first workaround was how I was working around this original, but I had forgotten about the second! After typing the above, I then looked at the implementation of this rule, which seems to already use types? And sure enough:
This would mean this no longer requires a breaking change. Since this rule already supports
These are in order of preference, since the 3rd option would be rather wide, but less specific. |
|
Alternatively, you can use native destructuring defaults to replace |
Wrong rule. There is some duplication of assertion rules which needs to be cleaned up (#501). This rule specifically exists to disallow an explicit cast of an object type (as explicit casts of object types applies more relaxed rules than an implicit type cast, possibly leading to bugs). @j-f1 - every time I try this - it doesn't work for me... |
Useful to know - thanks 👍
Gosh darn it. I checked it like three times to make sure - sorry guys! @j-f1 destructuring is not the recommended way of doing this in TS, as it causes a performance hit (since the default values are evaluated every call). |
Since the
Do you have evidence that this is a significant performance hit? Not destructuring sounds kind of like premature optimization. |
If your app is concerned with the performance hit of initialising 1 string per render, then your app must be perfectly coded, and is in the 0.0001% of cases; you should be commended on how performant your app. Sarcasm aside, default values for props is the recommended way of doing it as it optimises for developer experience in exchange for a very, very negligable perf hit. Speaking from first-hand experience, the Facebook codebase uses object destructuring default values in its functional components, and they operate at the scale of tens if not hundreds of thousands of components. Considering how perf-conscious they are (you should see the perf tooling that gets automatically run and reported for every PR - every team has size and render budgets), I think that it's fine. If you're worried about the cost of initialising a string, feel free to rejig your components a bit so it's a prop lookup instead (which should more or less only cost 1 cycle for the memory lookup): const defaultProps = { myProp: 'hello world' };
const Comp = ({ myProp = defaultProps.myProp }) => {
// ...
}As an aside as well that I thought of whilst writing the above. Using object destructuring allows you to take advantage of performance tricks provided by the engine. (If you're transpiling destructuring statements) You don't know for certain that the string will be re-evaluated every time, nor do you know the tricks the engine has done under the hood to decide if it should use the default value. Where as with default props you know that the string was evaluate once, but you know the exact algorithm react is using every render, which has its own perf considerations and can only be optimised so much by the engine. Object.keys(defaultProps).forEach(k => {
if (!k in prop[key]) {
prop[key] = defaultProps[key];
}
}); |
Repro
There is no way to directly annotate the type of a function property.
A very common use case of
aswith a function property is the ReactdefaultPropsproperty:It would be reasonable to support allowing
asfor objects when they're being assigned to function properties.The text was updated successfully, but these errors were encountered: