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 ESLint-style config options #319

Open
j0hnm4r5 opened this issue May 10, 2018 · 4 comments
Open

Use ESLint-style config options #319

j0hnm4r5 opened this issue May 10, 2018 · 4 comments
Labels
💵 Funded on Issuehunt This issue has been funded on Issuehunt enhancement help wanted

Comments

@j0hnm4r5
Copy link

j0hnm4r5 commented May 10, 2018

Issuehunt badges

Problem

Issue is described here: https://gitter.im/xojs/Lobby?at=5af49381bd10f34a68f18c4d

TLDR; XO's config options are not the same format as ESLint's, leading to increased risk for errors/bugs/confusion, as well as being less powerful (compare XO's "global": [] to ESLint's "global": {}, for example).

Proposal

Use the conventions defined in ESLint's config settings, i.e., objects instead of arrays and uniform naming "env", not "envs".

There is a $60.00 open bounty on this issue. Add more on Issuehunt.

@pvdlg
Copy link
Contributor

pvdlg commented May 11, 2018

The advantage of having global and env options defined as objects is to allow to set some to false. This is useful when extending shareable configuration: if a shareable config you extend define var1 as global and you don't want it, you can disable it in your config by setting it to false.

XO configuration is not extendable. However it allows to extend ESLint shareable config, so there might be cases where one want to disable some global or env values.

On of the challenge is that those 2 options can be set via CLI. That means we would need to use dot notation:

xo --env.foo --env.bar false
// => {env: {foo: true, bar: flase}

We will also have to maintain compatibility with the current array format for backward compatibility and as ESLint supports it as well. So that would require a bit of transformation.

Does that concerns only the env and global options or did you find another one in the same situation?

@j0hnm4r5
Copy link
Author

The only two I was using were env/envs and global, so that's all I can speak for, unfortunately.

The advantage of having global and env options defined as objects is to allow to set some to false.

Just to be clear, the behavior now is essentially that:

"xo" : {
    "globals": [
        "foo"
    ]
}

turns directly into:

"eslintConfig": {
    "globals": {
        "foo": true    
    }
}

@sindresorhus
Copy link
Member

As long as the old format with arrays is preserved, I don't mind supporting this. I went with array as I never needed to set the writability of globals.

@IssueHuntBot
Copy link

@IssueHunt has funded $60.00 to this issue.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💵 Funded on Issuehunt This issue has been funded on Issuehunt enhancement help wanted
Projects
None yet
Development

No branches or pull requests

4 participants