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

Unsafe scopes are forbidden from array's map() function #54

Closed
kctang opened this issue Jul 27, 2018 · 6 comments
Closed

Unsafe scopes are forbidden from array's map() function #54

kctang opened this issue Jul 27, 2018 · 6 comments
Labels

Comments

@kctang
Copy link

kctang commented Jul 27, 2018

With rxjs-tslint-rules v4.7.0, and "rxjs-no-unsafe-scope": true, I am getting this rather unexpected "Unsafe scopes are forbidden".

Funny thing is, this whole chunk is NOT about observables.. just normal array map() so I would expect the rules to not take effect.

I only happens when the variable was destructured from parameter (e.g. notOk_mapAnArrayFromParam()). It is ok when used in ok_mapAnArray().

Maybe a bug?

  // this is ok
  ok_mapAnArray () {
    const q = '123'
    return [ 1, 2, 3 ].map(val => {
      // "q" is ok
      return `${q}-${val}`
    })
  }

  notOk_mapAnArrayFromParam (input: { q: string }) {
    const { q } = input
    return [ 1, 2, 3 ].map(val => {
      // "q" is NOT ok - complain about "Unsafe scopes are forbidden"
      return `${q}-${val}`
    })
  }
@cartant
Copy link
Owner

cartant commented Jul 28, 2018

Yes. This is a bug. The fact that it's const should see it deemed safe. Also, I will have to check the implementation, as I thought it would only be checking observable-related functions - I added a general version of the rule - for any callback - to tslint-etc. It might only be checking the function name and RxJS has a map operator.

@cartant cartant added the bug label Jul 28, 2018
@cartant
Copy link
Owner

cartant commented Jul 28, 2018

Yeah, the implementation looks for RxJS functions by matching names only - not return types. So map is a match. This should probably be fixed, but I think the const destructuring thing is higher priority.

This probably affects other rule implementations, too. Create isObservableFactory/Operator so that custom functions are also supported.

cartant added a commit that referenced this issue Jul 31, 2018
@cartant
Copy link
Owner

cartant commented Jul 31, 2018

BTW, thanks for opening such details issues. They're really easy to convert into failing tests. Much appreciated.

@cartant
Copy link
Owner

cartant commented Aug 1, 2018

The const part of this issue should be fixed in 4.7.2 and I've opened a separate issue for the matching-by-name-only behaviour.

@vitalyiegorov
Copy link

@cartant Do we have any updates with Array.map?

@cartant
Copy link
Owner

cartant commented Jun 4, 2019

No. The issue referenced above is still open.

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

No branches or pull requests

3 participants