eslint-plugin-json icon indicating copy to clipboard operation
eslint-plugin-json copied to clipboard

Enable processing of further plugins

Open stephtr opened this issue 4 years ago • 7 comments

This is an approach to fix #38, to enable using this plugin in combination with eslint-plugin-prettier. This change modifies JSON code to be valid JS (by embedding it into a JSON.stringify function, which won't get executed) and furthermore enables autofixing.

stephtr avatar Mar 22 '20 14:03 stephtr

damn, I wasn't supposed to push on your branch without asking 🤦‍♂ sorry about that @stephtr :/

AdrieanKhisbe avatar Mar 23 '20 22:03 AdrieanKhisbe

Codecov Report

Merging #39 into master will decrease coverage by 1.35%. The diff coverage is 95.83%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #39      +/-   ##
===========================================
- Coverage   100.00%   98.64%   -1.36%     
===========================================
  Files            1        1              
  Lines           51       74      +23     
===========================================
+ Hits            51       73      +22     
- Misses           0        1       +1     
Impacted Files Coverage Δ
src/index.js 98.64% <95.83%> (-1.36%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1bbe2bc...72e2bdd. Read the comment docs.

codecov-io avatar Mar 23 '20 22:03 codecov-io

that were supposed to be on a branch on that repo, and not yours

This way it's also perfectly fine!

Ah thanks, I forgot that the rules of this plugin don't use the output of the preprocessor. That affects the failing test and the error mapping.

I changed the eol regex from /([\n\r]{0,2})?$/ to /([\n\r]*)$/. I think it makes more sense to convert {}\n\n to JSON.stringify({})\n\n than to JSON.stringify({}\n)\n.

I'll add a test case together with Prettier soon.

stephtr avatar Mar 24 '20 20:03 stephtr

@stephtr Cool, I'll wait until you add the prettier integration test then. :)

(by the way, prettier might want you to add a semi column to the preprocessorTemplate, and maybe a trailing newline)

AdrieanKhisbe avatar Mar 24 '20 21:03 AdrieanKhisbe

(by the way, prettier might want you to add a semi column to the preprocessorTemplate, and maybe a trailing newline)

Prettier recognizes the file type via the file extension. In case of JSON files it doesn't add semicolons or singleQuotes (neither does eslint). We need the preprocessorTemplate stuff only to get through eslint, fortunately it doesn't affect Prettier, even though it's illegal JSON. That's by the way a reason why I'm thinking whether we should "hide" this PR behind a config flag, then there also wouldn't be a breaking change.

Concerning the trailing newline: If the original file contains a trailing newline, it gets appended to the template. If not and Prettier adds it, the fix gets applied to the original file, which is exactly the behavior we want.

stephtr avatar Mar 24 '20 23:03 stephtr

Hey guys, is this any soon to be merged? We tried to integrate to our homebacked config, but because of this issue, we had to turn it off.

dentuzhik avatar Dec 07 '20 12:12 dentuzhik

Hey folks, I have been trying to look through the change and understand its potential impact. I have generally been nervous about making significant changes that can potentially break existing users. Honestly, the eslint ecosystem has changed a lot and I have not kept up with it which makes me extra nervous.

That being said, I do understand that as the ecosystem changes, a different solution might be more valuable. Recently someone shared this alternative project that seems to provide a JSON parser for ESLint with built in support for auto-fixing some issues: https://github.com/zeitport/eslint-parser-json. I think it might be worth checking if this solution fits your use case better.

azeemba avatar Dec 09 '20 01:12 azeemba