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

Use cwd for create eslint fix #242

Closed

Conversation

lewisl9029
Copy link
Contributor

This seems to fix prettier/prettier-atom#505

Previously we weren't specifying a cwd option for eslint in createEslintFix, so the working directory defaulted to where prettier-atom invoked the function, i.e. somewhere in the atom config directory. This ensures we infer the cwd from the filePath as we do in getESLintConfig here: https://github.com/prettier/prettier-eslint/blob/master/src/index.js#L227

The change seems to have broken this test assertion expecting 2 calls to readFileSync: d9d190e#diff-8a18675f0cf6827722b5896d753397b9L356

I updated the test here but wasn't fully sure of the implications. Let me know if this is problematic. Would love to hear other suggestions on how to move forward with this bugfix.

Unsure about the implications of this change in behavior. Anything I 
should double check?
@lewisl9029
Copy link
Contributor Author

@zimme mind taking a look at this one?

It's meant to fix the most popular issue on prettier-atom with 33 👍 s and 39 comments, affecting a lot of people: prettier/prettier-atom#505

And the fix itself is fairly small: https://github.com/prettier/prettier-eslint/pull/242/files#diff-1fdf421c05c1140f6d71444ea2b27638R173

Thanks!

Copy link

@jjosef jjosef left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@jjosef
Copy link

jjosef commented Sep 24, 2019

Is anyone maintaining this project? This is a serious issue.

@SebiTimeWaster
Copy link

is there maybe a usable fork of this project that has this fix?

@SebiTimeWaster
Copy link

image

there was no activity in 1 month on this project, i think it is dead.

@jjosef
Copy link

jjosef commented Sep 26, 2019

Probably a good idea. I am unable to at the moment but would be nice to see this happen

@zimme
Copy link
Member

zimme commented Sep 26, 2019

Will look into this later this week when I'll look into the eslint 6 stuff, if my family life permits me some spare time

@louisscruz
Copy link

Any update on this? I've just verified that this does in fact solve prettier/prettier-atom#505.

@digitalmaster
Copy link

Should we fork this plugin?

@j-f1
Copy link
Member

j-f1 commented Nov 12, 2019

cc @prettier/eslint

@zimme
Copy link
Member

zimme commented Nov 17, 2019

Merged in 6d4fb22 with some changes.

Please test 9.0.1 and see if this is solved.

@zimme zimme closed this Nov 17, 2019
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.

Failed to load plugin 'prettier' declared in 'CLIOptions': Cannot find module 'eslint-plugin-prettier'
8 participants