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

Add settings and parser options #142

Merged
merged 2 commits into from
Oct 11, 2016
Merged

Add settings and parser options #142

merged 2 commits into from
Oct 11, 2016

Conversation

wmertens
Copy link
Contributor

Closes #138.

@wmertens
Copy link
Contributor Author

Weird thing though, when I use this to set the import/resolver, it makes it into the config that is generated for ESLint, but the import plugin only gets the rule set by the plugins extend.

generated config:

{ rules: 
   { strict: 0,
     quotes: 0,
     eqeqeq: [ 2, 'allow-null' ],
     'no-eq-null': 0,
     'operator-linebreak': [ 2, 'after', [Object] ],
     'brace-style': [ 2, '1tbs', [Object] ],
     'babel/arrow-parens': 0,
     'comma-dangle': [ 2, 'always-multiline' ],
     'react/jsx-closing-bracket-location': [ 2, 'tag-aligned' ],
     'react/prop-types': 1,
     'react/prefer-stateless-function': 1,
     'prefer-template': 0,
     semi: [ 2, 'never' ],
     'semi-spacing': [ 2, [Object] ] },
  settings: { 'import/resolver': { webpack: [Object] } },
  globals: [ '$', 'info', 'dbg', 'views', '_dict', 'dict', 'lang' ],
  envs: [ 'browser', 'mocha', 'webpack' ],
  plugins: [ 'react', 'babel' ],
  extends: [ 'xo-react' ],
  useEslintrc: false,
  cache: true,
  cacheLocation: '/Users/wmertens/.xo-cache/',
  baseConfig: 
   { extends: 
      [ 'xo/esnext',
        '/Users/wmertens/.../src/node_modules/xo/config/plugins.js',
        '/Users/wmertens/.../src/node_modules/eslint-config-xo-react/index.js' ] },
  esnext: true,
  semicolon: false,
  cwd: '/Users/wmertens/.../src',
  init: false,
  compact: false,
  stdin: false,
  fix: false,
  open: false,
  ignores: 
   [ '**/node_modules/**',
     '**/bower_components/**',
     'coverage/**',
     '{tmp,temp}/**',
     '**/*.min.js',
     '**/bundle.js',
     'fixture{-*,}.{js,jsx}',
     'fixture{s,}/**',
     '{test,tests,spec,__tests__}/fixture{s,}/**',
     'vendor/**',
     'dist/**' ] }

settings that the import plugin receives:

{ 'import/extensions': [ '.js' ] }

Does eslint not properly merge the settings?

@wmertens
Copy link
Contributor Author

Meep?

Copy link
Member

@sindresorhus sindresorhus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add it to the readme?

@@ -73,6 +75,12 @@ test('buildConfig: rules', t => {
t.deepEqual(config.rules, rules);
});

test('buildConfig: settings', t => {
const settings = {'import/resolver': 'webpack'};
const config = manager.buildConfig({settings: settings});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const config = manager.buildConfig({settings});

@sindresorhus
Copy link
Member

Weird thing though, when I use this to set the import/resolver, it makes it into the config that is generated for ESLint, but the import plugin only gets the rule set by the plugins extend.

// @jfmengels in case you have any idea what could cause this.

@jfmengels
Copy link
Contributor

Sorry, don't have a clue 😕

@wmertens
Copy link
Contributor Author

Done

@sindresorhus
Copy link
Member

Ok, figured it out. It's because we pass these option to the ESLint Node.js API, not as normal config (since it's generated). The ESLint Node.js API doesn't yet have a settings option. @wmertens Can you open an issue on ESLint about adding that?

@wmertens
Copy link
Contributor Author

@sindresorhus it turns out you're supposed to pass the config via baseconfig

@sindresorhus
Copy link
Member

@wmertens Ah ok. Makes sense. Can you update to use baseConfig instead? (See extends for example).

@sindresorhus sindresorhus changed the title options-manager: add "settings" support Add settings option Oct 3, 2016
@sindresorhus
Copy link
Member

No pressure on this, but I plan on doing a new release in a few days, so this needs to be updated if you want it included ;)

@wmertens
Copy link
Contributor Author

wmertens commented Oct 3, 2016

Noted, will try ☺

On Mon, Oct 3, 2016, 9:27 AM Sindre Sorhus notifications@github.com wrote:

No pressure on this, but I plan on doing a new release in a few days, so
this needs to be updated if you want it included ;)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#142 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADWlpA02K3OuJc6rb7lECpqddT3yRjYks5qwK5egaJpZM4J8uT6
.

@sindresorhus
Copy link
Member

Last chance. Tomorrow is release day ;)

@wmertens
Copy link
Contributor Author

@sindresorhus Alright, this should do the trick…

@sindresorhus sindresorhus changed the title Add settings option Add settings and parser options Oct 11, 2016
@sindresorhus sindresorhus merged commit 85c6094 into xojs:master Oct 11, 2016
@sindresorhus
Copy link
Member

Awesome. Glad we could land this before the release. Thank you @wmertens :)

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

Successfully merging this pull request may close these issues.

3 participants