Add xo/eslint-adapter and remove xoToEslintConfig#876
Add xo/eslint-adapter and remove xoToEslintConfig#876sindresorhus wants to merge 5 commits intomainfrom
xo/eslint-adapter and remove xoToEslintConfig#876Conversation
|
I just wanted to bring this to top level as a comment: I generally feel this change is a step backwards for xo. It simply makes it more obtuse to get prettier integration and react integration. I think file look ups and internal resolutions is always a brittle design (may work perfectly IDK, but that's besides the point). Exporting a function was dead simple in comparison and was much more flexible. The use case is also kind of silly, if you want editor integration with the cli, you should use our editor integrations. They already accomplish the goal of getting your editor to match your cli in a much more graceful way. Pros:
Cons:
Anyway, at best this PR is taking away some flexibility and not giving anything new. At worst this PR is taking away some flexibility, and not giving anything new. Why do it? As an alternative that I would fully support: I would propose we leave this adapter out entirely and put the full functionality of xoToEslintConfig into eslint-config-xo. I think it will take a few more boolean options, and achieve the same flexibility as xoToEslint
Would be simple to provide a migration guide for every use case that way that doesn't feel different to users. |
|
Another IE eslint editor integration looks for changes in eslint config files and re-lints when changes occur. It will no longer work like that for users of the adapter, they will need to manually restart eslint in the editor everytime a change is made to the xo config.
Not sure if this same thing would affect the eslint cli config caching. Not really sure if that occurs, but if it does somehow, or if they add that in the future, it would have similar issues. I would totally support simple renaming |
Hello! I've been advocating for this for years! 😁
It seems we're stuck on the misunderstanding of who this layer is for. Let me try again: if you do not use XO's CLI and IDE integration, you do not need this layer; use Now, if |
|
Yes - I think its a good point that the function Here is my gripe example: I am using eslint and and eslint editor integration only. My import xo from 'xo'
export default xo.xoToEslintConfig([{react: true, prettier: true}, ....otherOverrides ]);With this change, I now have to add an export default [{react: true, prettier: true}, ...anyOtherOverrides]AND I have to keep my eslint.config.js import config from 'xo/eslint-adapter'
export default config;This is confusing and a step backwards in my opinon. BUT I would be totally happy if I will be stuck in a weird in between where both my We are leaving the react and prettier users (me almost always, and likely a significant portion of other xo users who use xoToEslintConfig and prettier or react integration) in a bad spot with this change I feel. Its a worst of all world in my opinion. |
Yes, in my view This means more work is needed before |
Fixes #875