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 and extend eslint's config files instead of mapping it to XO's own schema #428

Closed
fregante opened this issue Feb 20, 2020 · 5 comments
Closed

Comments

@fregante
Copy link
Member

fregante commented Feb 20, 2020

Some options are XO-specific, like space: true, but many of them (including rules and such) don't need to be customized to XO's own format.

An example of this is #319. This issue wouldn't exist at all if, instead of having a xo.globals key in package.json, XO just used eslintConfig.globals

This is particularly painful when copying configuration from internet and then having to match it to XO.

This is fine, it's the same for both XO and eslint:

{
	rules: {
		"no-console": "off"
	}
}

But what about this? From: https://github.com/sveltejs/eslint-plugin-svelte3

{
    overrides: [
      {
        files: ["**/*.svelte"],
        processor: "svelte3/svelte3"
      }
    ]
}

eslint has that processor property, XO doesn't. I still haven't figured this out.


It's great that XO offers shortcuts for common configuration, but could it leave eslint's config in eslint's files?

This is an example package.json for a random project (no need to read it):

{
  "xo": {
    "space": true,
    "rules": {
      "import/no-unassigned-import": 0,
      "no-new": 0
    },
    "plugins": [
      "svelte3"
    ],
    "envs": [
      "browser",
      "webextensions"
    ],
    "parserOptions": {
      "ecmaVersion": 2019,
      "ecmaFeatures": {
        "jsx": false
      }
    },
    "extensions": [
      "svelte"
    ],
    "overrides": [
      {
        "files": ["**/*.svelte"],
        "processor": "svelte3/svelte3" /* Doesn't work */
      }
    ]
  }
}

95% of this isn't XO-specific. I reckon it should be written as:

{
  "xo": {
    "space": true,
    "extensions": [
      "svelte"
    ]
  },
  "eslintConfig": {
    "rules": {
      "import/no-unassigned-import": 0,
      "no-new": 0
    },
    "plugins": [
      "svelte3"
    ],
    "envs": [
      "browser",
      "webextensions"
    ],
    "parserOptions": {
      "ecmaVersion": 2019,
      "ecmaFeatures": {
        "jsx": false
      }
    },
    "overrides": [
      {
        "files": ["**/*.svelte"],
        "processor": "svelte3/svelte3"
      }
    ]
  }
}

Being a thinner wrapper around eslint frees XO from having to document overlapping features. Does XO's readme really need to document envs, globals, ignores, rules, plugins, extends, settings, and parser? Yep, that's most of them.

The CLI could do a similar job at mapping unrecognized flags back to eslint (risky but better than having no chance at all)


This is a pretty big change at this point, but ideal for a XO 1.0 nonetheless.

@sindresorhus
Copy link
Member

I feel like ESLint has way too many options and CLI flags and I don’t want to overload users with all that. I also see value in being able to add custom behavior to options when we can improve upon ESLint. I’m happy to add support for reading from eslintConfig in package JSON too, but I don’t want to remove the XO options.

@pvdlg
Copy link
Contributor

pvdlg commented Feb 25, 2020

What would be the point of splitting the XO only and the ESLint config?
Currently you can use any ESLint option in the XO config and it will be passed down to ESLint, even if it's an override.

With the example you gave:

{
   overrides: [
     {
       files: ["**/*.svelte"],
       processor: "svelte3/svelte3"
     }
   ]
}

I tested and the processor option is passed down to CLIEngine.

The only exceptions are envs and globals that can be configured either as an array or an object in ESLint by only as array in XO.
Other than those two you can just copy/paste any ESLint config into the XO config and it will work.

One more thing regarding file extension. XO since 0.27.0 glob the files first and then look for configuration files based on the files being linted. That mean if you call xo without specifying a glob pattern or the --extension option it will only retrieve ts, js, tsx and jsx files.
In your case you would have to call xo with xo --extension svelte

@fregante
Copy link
Member Author

fregante commented Feb 26, 2020

I already specified extensions in the config and that still doesn’t work for me. I’ll open an issue specifically for svelte.

What would be the point of splitting the XO only and the ESLint config?
Currently you can use any ESLint option in the XO config and it will be passed down to ESLint, even if it's an override.

You answered yourself. That’s not documented, why would anyone expect that XO’s options are also silently ESLint options? The schema doesn’t seem to match.

Wouldn’t it make more sense to make this more explicit by having the user put ESLint options in ESLint’s config files?

XO is meant to provide defaults and make linting easier for the end user, but the “thicker” the layer around ESLint the more documentation you have to write and keep up to date around it, or else you’ll end up like this.

The only drawback I see is having 2 config keys inside package.json instead of 1, but if the user trusts XO then probably they don’t need a XO key in there at all. Congrats, you achieved “no config needed” status

I feel like ESLint has way too many options and CLI flags and I don’t want to overload users with all that.

You can still suggest a handful of options without making it look like they’re specific to XO.

@fregante
Copy link
Member Author

fregante commented Aug 6, 2022

What would be the point of splitting the XO only and the ESLint config?

With the upcoming Flat Configmageddon I think reducing your XO-specific config would be your first priority. I think your choices are:

  • leave xo.config unchanged and further widen the gap that this issue and #447 shows
  • rework xo.config trying to match the new config format
  • get rid of everything except XO-native config like --space, --prettier and so on.

Related:

@fregante
Copy link
Member Author

Closing as non-actionable. Further discussion available in:

This issue can be considered folded into:

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants