-
- Notifications
You must be signed in to change notification settings - Fork 58
feat: added the prefer-svelte-reactivity
rule #1151
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
feat: added the prefer-svelte-reactivity
rule #1151
Conversation
🦋 Changeset detectedLatest commit: 0996bb0 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 |
d061a99
to 6999c90
Compare Try the Instant Preview in Online PlaygroundInstall the Instant Preview to Your Local
Published Instant Preview Packages:
|
5cede00
to f44a65c
Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to add tests for .svelte.js files also.
...lint-plugin-svelte/tests/fixtures/rules/prefer-svelte-reactivity/invalid/date01-input.svelte Show resolved Hide resolved
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 :/ |
@marekdedic What have you tried? Have you added
|
7603712
to d1230de
Compare
That's exactly what I tried, but it didn't work :( |
18dad7e
to dcf3e34
Compare Ok, I was adding it in a different place and the value got overriden, thanks :) |
...lint-plugin-svelte/tests/fixtures/rules/prefer-svelte-reactivity/invalid/date01-input.svelte Show resolved Hide resolved
dcf3e34
to 1319f62
Compare 30e17ee
to 8e74c27
Compare 169791a
to af1cb8f
Compare packages/eslint-plugin-svelte/src/rules/prefer-svelte-reactivity.ts Outdated Show resolved Hide resolved
af1cb8f
to 1689df3
Compare 23c8253
to 586dca9
Compare I have re-implemented the PR so that it checks all the property calls and only reports if the variable is mutated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Looks almost good to me, but I have two comments.
packages/eslint-plugin-svelte/src/rules/prefer-svelte-reactivity.ts Outdated Show resolved Hide resolved
586dca9
to 064f100
Compare 064f100
to 0996bb0
Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
@baseballyama Just to be sure, could you please check this PR? |
I will review it within a few days.👍 |
@baseballyama Gentle reminder :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for late review!
No worries, I presume you also do this in your free time :) |
Closes #1071