Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] New lint rule to disallow referential type literals in object destructuring params #2847

Open
cyan33 opened this issue Oct 30, 2020 · 2 comments

Comments

@cyan33
Copy link
Contributor

cyan33 commented Oct 30, 2020

Hi folks! We recently just landed a new react lint rule in Facebook and wondered if it could be more beneficial to contribute it upstream. Before opening a PR here's some background and document. Just want to see how y'all feel about it.

The lint rule is to disable scenarios when:

  • it's functional component
  • it's using the object destructuring syntax to set default props
  • and the default props is a referential typed literals, e.g. functional expression, object / array literal, etc.

To put it in code:

function Foo({
  number = 3, // valid
  data = [], // invalid  
}) {}

Here's a full document to explain the rationale (Full credit to @bradzacher for writing it):


Overview

Certain values (like arrays, objects, functions, etc) are compared by reference instead of by value. This means that, for example, whilst two empty arrays represent the same value - to the JS engine they are distinct and inequal as they represent references to different things in memory.

When using object destructuring syntax you can set the default value for a given property if it does not exist. If you set the default value to one of the values that is compared by reference, it will mean that each time the destructure is executed the JS engine will create a new, non-equal value in the destructured variable.

In the context of a React functional component's props argument this means that each render the property has a new, distinct value. When this value is passed to a hook as a dependency or passed into a child component as a property react will see this as a new value - meaning that a hook will be re-evaluated, or a memoized component will rerender.

This obviously destroys any performance benefits you get from memoisation. Additionally in certain circumstances this can cause infinite rerender loops, which can often be hard to debug.

It's worth noting that primitive literal values (string, number, boolean, null, and undefined) are compared by value - meaning these are safe to use as inlined default destructured property values.

How to Fix it

The easiest way is to use a referencing variable instead of using the literal values, e.g:

const emptyArray = [];

function Component({
  items = emptyArray,
}) {}

Examples

❌ Examples of invalid code for this rule:

function Component({
  items = [],
}) {}

const Component = ({
  items = {},
}) => {}

const Component = ({
  items = () => {},
}) => {}

✅ Examples of valid code for this rule:

const emptyArray = [];

function Component({
  items = emptyArray,
}) {}

const emptyObject = {};
const Component = ({
  items = emptyObject,
}) => {}

const noopFunc = () => {};
const Component = ({
  items = noopFunc,
}) => {}

// primitive literals are all compared by value, so are safe to be inlined
function Component({
  num = 3,
  str = 'foo',
  bool = true,
}) {}
@ljharb
Copy link
Member

ljharb commented Oct 30, 2020

Thanks, this seems like a great addition.

@cyan33
Copy link
Contributor Author

cyan33 commented Oct 30, 2020

I can just copy the rule and submit the PR real quick :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants