feat: added the prefer-svelte-reactivity rule
#1151
Conversation
🦋 Changeset detectedLatest commit: 7603712 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
marekdedic
force-pushed
the
prefer-svelte-reactivity
branch
from
d061a99 to
6999c90
Compare
Try the Instant Preview in Online PlaygroundInstall the Instant Preview to Your LocalPublished Instant Preview Packages:
|
marekdedic
force-pushed
the
prefer-svelte-reactivity
branch
from
6999c90 to
5cede00
Compare
marekdedic
force-pushed
the
prefer-svelte-reactivity
branch
from
5cede00 to
f44a65c
Compare
| <script> | ||
| const variable = new Date(8.64e15); | ||
| </script> | ||
|
|
||
| {variable} |
There was a problem hiding this comment.
I don’t think this needs to be reported. If there’s no reassignment, it shouldn’t need to be reactive.
There was a problem hiding this comment.
It's not about reassignment, it's about internal changes. I'd prefer to be this overly-broad. The alternative is tracking calls to every method that modifies the object (I count 16 on Date alone) or writing to any property.
On top of that, once something is exported, it should be reported as well (mostly relevant for .svelte.ts files)
I am not sure it's worth it, what do you think?
There was a problem hiding this comment.
What I’m saying is the same thing.
// It should be SvelteMap because there is internal changes.
const foo = new Map();
foo.set("a", 0);
// It should not be reported because there is no internal changes.
const bar = new Map();On top of that, once something is exported, it should be reported as well (mostly relevant for .svelte.ts files)
If we think of svelte.js as “reactive files,” it should be fine to report aggressively.
Yes, I wanted to ask about whether that's something we're set up to do, I couldn't find any such tests on the repo... |
|
@ota-meshi Can you please take a look at the test file? I tried the solution from eslint-community/eslint-utils#249 (comment), but it didn't work for me :/ |
Closes #1071
Blocked by eslint-community/eslint-utils#249