Skip to content

Object.keys + sort in 'no-side-effects-in-computed-properties' #306

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

Closed
frandiox opened this issue Dec 27, 2017 · 5 comments
Closed

Object.keys + sort in 'no-side-effects-in-computed-properties' #306

frandiox opened this issue Dec 27, 2017 · 5 comments

Comments

@frandiox
Copy link

Tell us about your environment

  • ESLint Version: 4.12.0
  • eslint-plugin-vue Version: 4.0.0-beta.4
  • Node Version: 8.8.1

Please show your full configuration:

module.exports = {
  root: true,
  parserOptions: {
    parser: 'babel-eslint',
    sourceType: 'module'
  },
  env: {
    browser: true,
  },
  extends: ['airbnb-base', 'plugin:vue/recommended'],
  // check if imports actually resolve
  'settings': {
    'import/resolver': {
      'webpack': {
        'config': 'build/webpack.base.conf.js'
      }
    }
  },
  // add your custom rules here
  'rules': {
    // don't require .vue extension when importing
    'import/extensions': ['error', 'always', {
      'js': 'never',
      'vue': 'never'
    }],
    // allow optionalDependencies
    'import/no-extraneous-dependencies': ['error', {
      'optionalDependencies': ['test/unit/index.js']
    }],
    // allow debugger during development
    'no-debugger': process.env.NODE_ENV === 'production' ? 2 : 0,
    'no-confusing-arrow': 'off',

    // VUE
    'vue/max-attributes-per-line': [2, {
      'singleline': 3,
      'multiline': {
        'max': 1,
        'allowFirstLine': false
      }
    }],
    'no-param-reassign': [
      'error',
      {
        'props': true,
        'ignorePropertyModificationsFor': [
          'state',
          'acc',
          'e',
          'ctx',
          'req',
          'request',
          'res',
          'response',
          'result',
        ]
      }
    ],
  }
}

What did you do? Please include the actual source code causing the issue.

computed: {
  first() {
    return {};
  },
  second() {
    return Object.keys(this.first).sort();
  },
},

What did you expect to happen?

sort changes the array in place, but Object.keys is creating a new array so I think it shouldn't be considered as a side effect.

What actually happened? Please include the actual, raw output from ESLint.

  ✘  https://google.com/#q=vue%2Fno-side-effects-in-computed-properties  Unexpected side effect in "second" computed property            
  src/components/TRPreviewList.vue:68:14
        return Object.keys(this.first).sort();
                ^
@michalsnik
Copy link
Member

Hello @frandiox Thanks for posting this issue. I know where the problem is :) I'll try to work on this tomorrow evening

@michalsnik
Copy link
Member

Done in #308

@malles
Copy link

malles commented Feb 1, 2018

I still have the same false detection with this code snippet:

                this.$refs.invoices.invoices.forEach(invoice => attachments.push({
                    type: 'invoice' + invoice.invoice_number,
                    path: 'invoicemakerInvoices:/' + invoice.pdf_file,
                    name: invoice.pdf_filename,
                }));

@frandiox
Copy link
Author

frandiox commented Feb 1, 2018

I also saw another false positive with something like return [this.something].shift(); or return [this.a, this.b, this.c].sort().shift();.

@nash403
Copy link

nash403 commented Feb 13, 2018

I had the same issue with the es6 spread operator: return [...this.myArray].sort().

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

4 participants