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

fix(prefer-to-have-style): do not offer invalid autofix for computed accessors #306

Merged
merged 2 commits into from
Jun 4, 2023

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Jun 4, 2023

What:

Changes prefer-to-have-style to not give an autofix if a variable is being used to access style.

Why:

Because it generates an invalid autofixer.

Note that we could probably do at least a suggestion (or two), but at this point I'm not too sure of the semantics and currently this is creating invalid code - if someone wants to do a followup PR that improves this rule, I'd be happy to review it.

How:

By not setting fix if the node is an Identifier.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

Resolves #265
Resolves #286

(note I've purposely not formatted my code to test a theory)

@G-Rath G-Rath added the bug Something isn't working label Jun 4, 2023
@codecov
Copy link

codecov bot commented Jun 4, 2023

Codecov Report

Merging #306 (547d68f) into main (96c364a) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #306   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          581       584    +3     
  Branches       165       168    +3     
=========================================
+ Hits           581       584    +3     
Impacted Files Coverage Δ
src/rules/prefer-to-have-style.js 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@G-Rath
Copy link
Collaborator Author

G-Rath commented Jun 4, 2023

@MichaelDeBoey fyi I'm pretty sure that kcd-scripts lint is doing absolutely nothing, so neither eslint or prettier is being properly run against this repo.

@MichaelDeBoey
Copy link
Member

@G-Rath Would you be able to look into the problem?

@MichaelDeBoey MichaelDeBoey merged commit b38b8ea into main Jun 4, 2023
62 checks passed
@MichaelDeBoey MichaelDeBoey deleted the fix-autofix branch June 4, 2023 19:51
@G-Rath
Copy link
Collaborator Author

G-Rath commented Jun 4, 2023

@MichaelDeBoey ideally no as I'm not really a big proponent for these sort of things, but I can spend a couple of minutes digging to at least try to make an issue if not a PR.

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

🎉 This PR is included in version 5.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@G-Rath
Copy link
Collaborator Author

G-Rath commented Jun 4, 2023

@MichaelDeBoey ok so it looks like linting is working, it just doesn't run prettier - I think you'll need to take it from here because it looks like eslint-config-kentcdodds is outright missing eslint-plugin-prettier even though it has eslint-config-prettier, and I don't know if that's intentional or if there will need to be any other changes or what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[prefer-to-have-style] error when attempting to fix with computed access of el.style
2 participants