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

Leverage eslint-plugin-compat with `this.project.targets` #158

Open
cibernox opened this Issue Mar 14, 2017 · 10 comments

Comments

Projects
None yet
6 participants
@cibernox
Contributor

cibernox commented Mar 14, 2017

Some context: https://github.com/amilajack/eslint-plugin-compat

This eslint plugin is gaining popularity. The idea is to customize the rules based on the target browsers, warning when the user attempts to use a feature that is not present across the board for the entire support matrix, unless it has been polyfilled.

Examples of those include:

  • fetch
  • MutationObservers
  • WebAssembly

This issue intends to be a placeholder to discuss wether we want to bake this into this plugin or develop it as a separate addon, and track its implementation if needed.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Mar 14, 2017

Collaborator

I'm generally in favor, but I think the extent of things here would be to update the default .eslintrc.js file in the blueprint.

We should experiment with the config required, but I suspect something like this should suffice:

const targets = require('config/targets');

module.exports = {
  root: true,
  parserOptions: {
    ecmaVersion: 6,
    sourceType: 'module'
  },
  extends: 'eslint:recommended',
  env: {
    browser: true
  },
  plugins: ['compat'],
  rules: {
    'compat/compat': [2, targets.browsers]
  }
};

Can someone test / confirm this?

Collaborator

rwjblue commented Mar 14, 2017

I'm generally in favor, but I think the extent of things here would be to update the default .eslintrc.js file in the blueprint.

We should experiment with the config required, but I suspect something like this should suffice:

const targets = require('config/targets');

module.exports = {
  root: true,
  parserOptions: {
    ecmaVersion: 6,
    sourceType: 'module'
  },
  extends: 'eslint:recommended',
  env: {
    browser: true
  },
  plugins: ['compat'],
  rules: {
    'compat/compat': [2, targets.browsers]
  }
};

Can someone test / confirm this?

@cibernox

This comment has been minimized.

Show comment
Hide comment
@cibernox

cibernox Mar 14, 2017

Contributor

I'll try to teste it once the first ember-cli 2.13-beta with support for targets goes out

Contributor

cibernox commented Mar 14, 2017

I'll try to teste it once the first ember-cli 2.13-beta with support for targets goes out

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Mar 14, 2017

Collaborator

The beauty of this is that all you have to have is a config/targets.js file for the above to work, no need to wait 😺

Collaborator

rwjblue commented Mar 14, 2017

The beauty of this is that all you have to have is a config/targets.js file for the above to work, no need to wait 😺

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Apr 19, 2017

Collaborator

We should confirm the above works properly now...

Collaborator

rwjblue commented Apr 19, 2017

We should confirm the above works properly now...

@SlyDen

This comment has been minimized.

Show comment
Hide comment
@SlyDen

SlyDen Apr 24, 2017

in my case I got such error:

Configuration for rule "compat/compat" is invalid:
	Value "ie 11,last 2 Edge versions,last 2 Chrome versions,last 2 Firefox versions,last 1 Safari versions" has more items than allowed.

though if I omitted targets.browsers, e.g.: 'compat/compat': 2 it was ok ...

SlyDen commented Apr 24, 2017

in my case I got such error:

Configuration for rule "compat/compat" is invalid:
	Value "ie 11,last 2 Edge versions,last 2 Chrome versions,last 2 Firefox versions,last 1 Safari versions" has more items than allowed.

though if I omitted targets.browsers, e.g.: 'compat/compat': 2 it was ok ...

@cafreeman

This comment has been minimized.

Show comment
Hide comment
@cafreeman

cafreeman Jul 14, 2017

Ditto on the above. Getting this error with latest ember-cli, eslint, etc.

Ditto on the above. Getting this error with latest ember-cli, eslint, etc.

@cafreeman

This comment has been minimized.

Show comment
Hide comment
@cafreeman

cafreeman Jul 14, 2017

Actually I guess it's slightly different:

Configuration for rule "compat/compat" is invalid:
	Value "last 2 Edge versions,last 2 Chrome versions,last 2 Firefox versions,last 2 Safari versions,last 2 iOS versions,last 2 ChromeAndroid versions" should NOT have more than 0 items.

Actually I guess it's slightly different:

Configuration for rule "compat/compat" is invalid:
	Value "last 2 Edge versions,last 2 Chrome versions,last 2 Firefox versions,last 2 Safari versions,last 2 iOS versions,last 2 ChromeAndroid versions" should NOT have more than 0 items.
@sandydoo

This comment has been minimized.

Show comment
Hide comment
@sandydoo

sandydoo Sep 26, 2017

Couldn't get eslint-plugin-compat to accept the browser targets via the rule and I'm not sure it currently supports that option. However, it does support specifying the targets in eslintrc using settings! Also had to provide a relative path for the require too.

Working config:

const targets = require('./config/targets');

module.exports = {
  root: true,
  parserOptions: {
    ecmaVersion: 6,
    sourceType: 'module'
  },
  extends: 'eslint:recommended',
  env: {
    browser: true
  },
  settings: {
    targets: targets.browsers
  },
  plugins: ['compat'],
  rules: {
    'compat/compat': 2
  }
};

Tested with:

ember-cli-eslint@4.2.0
eslint-plugin-compat@1.0.4

Couldn't get eslint-plugin-compat to accept the browser targets via the rule and I'm not sure it currently supports that option. However, it does support specifying the targets in eslintrc using settings! Also had to provide a relative path for the require too.

Working config:

const targets = require('./config/targets');

module.exports = {
  root: true,
  parserOptions: {
    ecmaVersion: 6,
    sourceType: 'module'
  },
  extends: 'eslint:recommended',
  env: {
    browser: true
  },
  settings: {
    targets: targets.browsers
  },
  plugins: ['compat'],
  rules: {
    'compat/compat': 2
  }
};

Tested with:

ember-cli-eslint@4.2.0
eslint-plugin-compat@1.0.4
@raido

This comment has been minimized.

Show comment
Hide comment
@raido

raido Mar 27, 2018

Passing targets through settings: {..} also worked for me.

raido commented Mar 27, 2018

Passing targets through settings: {..} also worked for me.

@raido

This comment has been minimized.

Show comment
Hide comment
@raido

raido Mar 27, 2018

Actually eslint-plugin-compat does not currently support everything, for example Object.assign will pass through without a notice. So use with caution.

raido commented Mar 27, 2018

Actually eslint-plugin-compat does not currently support everything, for example Object.assign will pass through without a notice. So use with caution.

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