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

feat(eslint-plugin-mobx): configurable default autofix annotation for makeObservable #3881

Merged
merged 4 commits into from
May 29, 2024

Conversation

kade-robertson
Copy link
Contributor

@kade-robertson kade-robertson commented May 21, 2024

Adds an option for the mobx/exhaustive-make-observable eslint rule to configure whether fields are annotated with true or false with the autofixer.

This option defaults to true if not present or an invalid value is received to maintain existing behavior.

Code change checklist

  • Added/updated unit tests -- No unit testing in the eslint package, but I have tested these changes against some other projects that use this rule.
  • Updated /docs. For new functionality, at least API.md should be updated
  • Verified that there is no significant performance drop (yarn mobx test:performance)

Note: It looks like some formatting rules have changed since the last time this file was touched so there's quite a lot of changes.

Closes #3876

Copy link

changeset-bot bot commented May 21, 2024

🦋 Changeset detected

Latest commit: 074c373

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-mobx Patch

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

@kade-robertson kade-robertson marked this pull request as ready for review May 21, 2024 13:55
@urugator
Copy link
Collaborator

Would you mind creating a PR that just runs prettier so we would merge it first? If not, it's completely fine, just pinpoint me to the changed lines, I assume it should be just 9, 80, 85.

@kade-robertson
Copy link
Contributor Author

Would you mind creating a PR that just runs prettier so we would merge it first? If not, it's completely fine, just pinpoint me to the changed lines, I assume it should be just 9, 80, 85.

I can do that -- if it needs to be applied to the whole project though it looks like about 35 files are going to be changed. For now I'll push some commits in a bit that separate out the initial prettier write step and the changes I made to make it more obvious.

@kade-robertson
Copy link
Contributor Author

See 1e99618 for the extent of actual changes.

@mweststrate
Copy link
Member

With additional changes LGTM, thanks for adding this!

@urugator urugator merged commit 44a5fe0 into mobxjs:main May 29, 2024
1 check passed
@github-actions github-actions bot mentioned this pull request May 31, 2024
@github-actions github-actions bot mentioned this pull request Jun 13, 2024
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.

[eslint-plugin-mobx] mobx/exhaustive-make-observable autofix behavior should be inversed
3 participants