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

v0.13.0 crashes on plugin shared config #88

Closed
jamestalmage opened this issue Mar 22, 2016 · 14 comments
Closed

v0.13.0 crashes on plugin shared config #88

jamestalmage opened this issue Mar 22, 2016 · 14 comments

Comments

@jamestalmage
Copy link
Contributor

jamestalmage commented Mar 22, 2016

I tried to use:

"overrides": [
  {
    "files": ["test.js"],
    "extends": ["plugin:ava/recommended"]
  }
]

And things crashed.

TypeError: Cannot read property 'charAt' of null
    at posix (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/path-is-absolute/index.js:4:13)
    at isFilePath (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/eslint/lib/config/config-file.js:80:12)
    at /Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/eslint/lib/config/config-file.js:344:20
    at Array.reduceRight (native)
    at Object.applyExtends (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/eslint/lib/config/config-file.js:338:28)
    at loadConfig (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/eslint/lib/config.js:63:37)
    at new Config (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/eslint/lib/config.js:171:44)
    at CLIEngine.executeOnFiles (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/eslint/lib/cli-engine.js:493:28)
    at runEslint (/Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/xo/index.js:67:16)
    at /Users/jamestalmage/WebstormProjects/async-task-pool/node_modules/xo/index.js:40:11

I am not sure if it is overrides specific

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

@jamestalmage
Copy link
Contributor Author

I am not sure if it is overrides specific

It does not appear to be

@sindresorhus
Copy link
Member

@jfmengels
Copy link
Contributor

jfmengels commented May 5, 2016

I'm getting the same error when I do this in the package.json:

  "xo": {
    "space": 2,
    "plugin": ["lodash-fp"],
    "extends": ["plugin:lodash-fp/recommended"]
  }

or with the cli

xo --plugin lodash-fp --extend plugin:eslint-plugin-lodash-fp/recommended

but it works fine when I go and modify xo configuration in the node_modules

module.exports = {
    plugins: [
        'lodash-fp',
        'no-use-extend-native',
        'import-order',
        'ava',
        'xo',
        'promise'
    ],
    extends: [
        'plugin:lodash-fp/recommended',
        'plugin:ava/recommended',
        'plugin:xo/recommended'
    ],
    rules: {
        'no-use-extend-native/no-use-extend-native': 2,
        'import-order/import-order': 2,
        'promise/param-names': 2
    }
};

(v0.15)

@Tapppi
Copy link

Tapppi commented Jun 22, 2016

https://github.com/sindresorhus/xo/blob/6ec33e9776bb0da6b79529969c1c32aa098f0efc/options-manager.js#L122-L136

v0.16

Just got this. If you try to use a plugin, it results in the options array passed to eslint having a null in the extends array (because resolve-from doesn't find it). Tried a few other ways, but couldn't get a plugin's recommended config to work in any way.. Any steps forward with this?

@sindresorhus
Copy link
Member

Any steps forward with this?

Opening an issue on ESLint asking them to expose the method for resolving extends, so we don't have to implement a "broken" version ourselves. https://github.com/eslint/eslint/issues

@abhisekp
Copy link
Contributor

Any steps backward with this to make it work i.e. any previous version of xo or eslint which can fix this issue?

@sindresorhus
Copy link
Member

ESLint issue opened: eslint/eslint#7328

@sindresorhus
Copy link
Member

The ESLint team says the underlying problem has been fixed in ESLint, so we should be able to rip out our custom resolve logic:

xo/lib/options-manager.js

Lines 254 to 282 in 510d02e

if (options.extends && options.extends.length > 0) {
// TODO: This logic needs to be improved, preferably use the same code as ESLint
// user's configs must be resolved to their absolute paths
const configs = options.extends.map(name => {
// Don't do anything if it's a filepath
if (pathExists.sync(name)) {
return name;
}
// Don't do anything if it's a config from a plugin
if (name.startsWith('plugin:')) {
return name;
}
if (!name.includes('eslint-config-')) {
name = `eslint-config-${name}`;
}
const ret = resolveFrom(options.cwd, name);
if (!ret) {
throw new Error(`Couldn't find ESLint config: ${name}`);
}
return ret;
});
config.baseConfig.extends = config.baseConfig.extends.concat(configs);
}

We need to add a lot of test to confirm everything is still working though.

@IssueHuntBot
Copy link

@issuehuntfest has funded $80.00 to this issue. See it on IssueHunt

@yaodingyd
Copy link

I think current resolve logic on shared config still don't include baseConfig.cwd

@yaodingyd
Copy link

Eslint has a rfc eslint/rfcs#7 to simplify the resolution of sharable configs. Maybe we wait on this first.

@sindresorhus
Copy link
Member

Yeah, probably smart.

@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@sindresorhus sindresorhus removed the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Jul 18, 2024
@sindresorhus
Copy link
Member

Closing as this will be fixed with #702

@sindresorhus sindresorhus closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2024
@sindresorhus
Copy link
Member

The bounty from this issue has moved to #702

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

7 participants