Add prefer-destructured-store-props rule

[tivac]

📖 Rule Details

This rule reports on directly accessing properties of a store containing an object. These usages can instead be written as a reactive statement using destructuring to allow for more granular change-tracking and reduced redraws in the component.

An example of the improvements can be see in this REPL

<script>
  /* eslint svelte/prefer-destructured-store-props: "error" */
  $: ({ foo } = $store)
</script>

<!-- ✓ GOOD -->
{foo}

<!-- ✗ BAD -->
{$store.foo}

[ota-meshi]

Could you add the following test case? I tested it and noticed a false positive.

<script>
  $: foo2 = $store.foo;
</script>

{foo2}

[ota-meshi]

Also, could you add the following test case? I think member access in reactive statements should probably be ignored.

<script>
  import store from "./store.js";
	
  let foo, bar
  $: {
    foo = $store.foo;
    bar = $store.bar;
  }
</script>

<div>
  foo: {foo + " " + Date.now()}
</div>
<div>
  bar: {bar + " " + Date.now()}
</div>

Ideally it would be nice to use reactive variables, but I think that could be checked with another new rule.

[tivac]

Could you add the following test case? I tested it and noticed a false positive.

<script>
  $: foo2 = $store.foo;
</script>

{foo2}

I don't see the false positive here, can you elaborate?

https://svelte.dev/repl/1d043d248c0c4200b809ca84242301f1?version=3.49.0

[tivac]

Also, could you add the following test case? I think member access in reactive statements should probably be ignored.

I think they should still trigger, but they shouldn't have suggestions. Added tracking of imported & defined names to not suggest if there would be a collision.

[ota-meshi]

I don't think you'll see any difference in render times between using destructuring and assigning values via member access.

https://svelte.dev/repl/897da31522c3410596f0dbc4916e5014?version=3.49.0

In my opinion, these member accesses should not be reported as the purpose of this rule is to reduce render times.
In addition to this rule, I think it makes sense to add a new rule that corrects member access to the destructuring writing style. The purpose of the new rules is to enforce stylistic conventions.
In my opinion, it makes sense to have separate rules because they serve different purposes.

What do you think?

[ota-meshi]

I just noticed that the REPL you provided in #205 seems to have incorrect results due to typo.

-	<code>direct = obj.a</code><br />{obj.a + " " + Date.now()}	
+	<code>direct = obj.a</code><br />{direct + " " + Date.now()}	

You wrote the following, but when I fix the typo there is no difference in the timestamps.

Notice how often the timestamp is changing on the "obj.a" or "direct = obj.a" versions vs the "({ a } = obj)" version? Each change in that timestamp indicates another redraw for the element.

[ota-meshi]

Looking at this result, in my opinion, prefer-reactive-destructuring may also be better split rules.

[ota-meshi]

The purpose of prefer-reactive-destructuring might be seen as enforcing stylistic conventions rather than reducing rendering times.

[ota-meshi]

If we split the rules, it doesn't seem necessary to add a stylistic rule.
The prefer-destructuring rule and the new svelte/prefer-reactive-destructuring rule seem to report them.

<script>
  /* eslint svelte/prefer-reactive-destructuring: error */
  $: foo2 = $store.foo;
</script>

<script>
/* eslint prefer-destructuring: error */

import store from "./store.js";
  
let foo, bar
$: {
  foo = $store.foo;
  bar = $store.bar;
}
</script>

prefer-destructuring DEMO

[marekvospel]

The first letter should be lowercase to be consistent with existing rules. (see #218)

[baseballyama]

@tivac What are the remaining tasks of the PR? I can do the remaining tasks if you need.

[tivac]

@tivac What are the remaining tasks of the PR? I can do the remaining tasks if you need.

@baseballyama Same as #205, it's just a stylistic rule since there's no redraw-limiting reason to do it after Ota caught my typo. 🤦🏻‍♂️

[tivac]

Sorry I've left these hanging for so long @ota-meshi, summer got busy and I haven't had much OSS time <3 Also realizing I based them off a false premise didn't help with motivation for getting them done, embarrassed I missed something like that.

[ota-meshi]

@tivac Don't worry about it. No need to rush. Someone can take over and help out if needed.

[ota-meshi]

I think the svelte/prefer-destructured-store-props rule probably needs to be changed to only report member accesses outside of <script>.
Other places where destructuring is preferred over member access can be reported by svelte/prefer-reactive-destructuring and prefer-destructuring as stylistic rules.

[baseballyama]

I think the svelte/prefer-destructured-store-props rule probably needs to be changed to only report member accesses outside of <script>.

It means, for example below code, should we report like "Please use $ ({ foo } = $store)"?

<script>
	import store from "./store.js";
</script>
<p>{$store.foo}</p>

[ota-meshi]

It means, for example below code, should we report like "Please use $ ({ foo } = $store)"?

That's right. I think users can reduce re-renders by doing so.