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

XO does not support @scoped shareable configs #479

Closed
tjrgg opened this issue May 16, 2020 · 3 comments · Fixed by #480
Closed

XO does not support @scoped shareable configs #479

tjrgg opened this issue May 16, 2020 · 3 comments · Fixed by #480

Comments

@tjrgg
Copy link
Contributor

tjrgg commented May 16, 2020

It seems that XO doesn't have support for @scoped shareable configs under the extends option.

XO checks to see if a name specified in the extends config option contains "eslint-config-", which means it doesn't properly recognize @scoped or @scoped/eslint-config as valid names for shareable configs (ESLint Docs). XO instead attempts to add "eslint-config-" to the front of the name, so you end up with eslint-config-@scoped or eslint-config-@scoped/eslint-config, which obviously aren't able to be resolved and result in a "Cannot find module" error.

@sindresorhus
Copy link
Member

Yes, see:

xo/lib/options-manager.js

Lines 335 to 336 in ca21492

// TODO: This logic needs to be improved, preferably use the same code as ESLint
// user's configs must be resolved to their absolute paths

I opened an issue regarding this a long time ago (eslint/eslint#7328), and they say it's fixed, but I'm honestly not sure what was fixed and what we need to do in XO to support this.

@tjrgg
Copy link
Contributor Author

tjrgg commented May 16, 2020

@sindresorhus I'm not sure that ESLint is the problem. It seems that XO just doesn't check for that case.

It seems to me that this should be able to be fixed by adding a condition to check to see if its scoped here: https://github.com/xojs/xo/blob/v0.30.0/lib/options-manager.js#L339-L350.

If the name is scoped, the module be resolved as is and not have the eslint-config- prefixed.

I plan on testing this today and if it works, I'll submit a PR.

@sindresorhus
Copy link
Member

The problem is that XO shouldn't need to do this kind of resolution at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants