Skip to content

feat: add fixer for derived-has-same-inputs-outputs #1163

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

Merged
merged 10 commits into from
Mar 29, 2025

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Mar 28, 2025

Simply replaces the param names with the correct ones since we're already aware of what they should be.

Depends on #1162

Copy link

changeset-bot bot commented Mar 28, 2025

🦋 Changeset detected

Latest commit: 5b85027

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-svelte Minor

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

Copy link
Contributor

github-actions bot commented Mar 28, 2025

Try the Instant Preview in Online Playground

ESLint Online Playground

Install the Instant Preview to Your Local

npm i https://pkg.pr.new/eslint-plugin-svelte@1163

Published Instant Preview Packages:

View Commit

@baseballyama
Copy link
Member

@43081j

According to the official ESLint documentation, the following is stated about fixers.

Best practices for fixes:
Avoid any fixes that could change the runtime behavior of code and cause it to stop working.
Make fixes as small as possible. Fixes that are unnecessarily large could conflict with other fixes, and prevent them from being applied.

https://eslint.org/docs/latest/extend/custom-rules#applying-fixes

In addition to this PR, it would also require changes to the places where the argument is used. In that case, we would need to check for any variable name conflicts, otherwise it would violate "Avoid any fixes that could change the runtime behavior of code and cause it to stop working.".

Therefore, implementing this as a fixer doesn’t seem appropriate. Would it be better to make it a suggestion at the very least?

@43081j
Copy link
Contributor Author

43081j commented Mar 29, 2025

similar to the other PR, i am aware of the best practices.

i just overlooked updating references to the parameters since we have no existing tests for those cases

if this becomes more complex than expected, it can be a suggestion. i'll see where i get to

Simply replaces the param names with the correct ones since we're
already aware of what they should be.
@43081j 43081j force-pushed the fixer-derived-inout branch from 397e365 to d1f13d0 Compare March 29, 2025 11:21
@43081j
Copy link
Contributor Author

43081j commented Mar 29, 2025

ok @baseballyama i've updated this to rename references of the variable in the current function scope

if you want me to change it into a suggestion once you've re-reviewed, let me know and ill update

EDIT just trying to fix a problem with the array pattern stuff...

@43081j 43081j force-pushed the fixer-derived-inout branch from d1f13d0 to 16099cb Compare March 29, 2025 11:25
@43081j 43081j force-pushed the fixer-derived-inout branch from 16099cb to 6a4bfc3 Compare March 29, 2025 11:41
Still figuring out what eslint is playing at.
@43081j
Copy link
Contributor Author

43081j commented Mar 29, 2025

this should be all good now

it uses suggestions and only provides them if there are no conflicting variable names

@baseballyama
Copy link
Member

Can you add a changeset?

@43081j 43081j force-pushed the fixer-derived-inout branch from 22de667 to 1710e44 Compare March 29, 2025 15:46
h;
});
});
derived([e, f], ([g, $f]) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shouldn't as the suggestions apply one at a time. this is listing the output of each individual suggestion

@baseballyama
Copy link
Member

it shouldn't as the suggestions apply one at a time. this is listing the output of each individual suggestion

Sorry! That's why I deleted the comment but you replied too fast😅

baseballyama
baseballyama previously approved these changes Mar 29, 2025
Copy link
Member

@baseballyama baseballyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
#1163 (comment) is last remaining comment.

Copy link
Member

@baseballyama baseballyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@baseballyama baseballyama merged commit d9b8604 into sveltejs:main Mar 29, 2025
17 checks passed
@43081j 43081j deleted the fixer-derived-inout branch March 29, 2025 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants