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

Error when encountering ArrowFunctionExpression in computed setter #424

Open
xg-wang opened this issue Mar 16, 2021 · 2 comments
Open

Error when encountering ArrowFunctionExpression in computed setter #424

xg-wang opened this issue Mar 16, 2021 · 2 comments

Comments

@xg-wang
Copy link

xg-wang commented Mar 16, 2021

For example:

  _list: computed('list.[]', {
    get() {
      return A(get(this, 'list').map((el) => el));
    },
    set: (key, value) => value
  }),

This code will throw

Transformation error (Cannot read property 'map' of undefined)
TypeError: Cannot read property 'map' of undefined
at Lines.Lp.join (~/.npm/_npx/41642/lib/node_modules/ember-native-class-codemod/node_modules/jscodeshift/node_modules/recast/lib/lines.js:867:12)

Chatted with @rwjblue, arrow function should be forbidden in computed properties: https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-arrow-function-computed-properties.md

The codemod should check ArrowFunctionExpression and throw a more meaningful error on this specific file

@elwayman02
Copy link

@xg-wang I don't think the codemod should throw an error at all, necessarily. Arrow functions should be forbidden in computed properties, but it is not an error to use them if you don't reference this. In fact, the lint rule you linked to specifically has a configuration option to allow arrow functions in computed properties where this isn't referenced. Since that is technically valid code, the codemod should not error but instead successfully transform the code.

@rwjblue
Copy link
Member

rwjblue commented Mar 16, 2021

IMHO the hierarchy here is that throwing a useful error is better than today and actually fixing the transform to function properly would be better than that.

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

No branches or pull requests

3 participants