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 null check in new-module-imports rule (again) #461

Merged
merged 1 commit into from
Aug 8, 2019
Merged

Add null check in new-module-imports rule (again) #461

merged 1 commit into from
Aug 8, 2019

Conversation

bmish
Copy link
Member

@bmish bmish commented Aug 8, 2019

Fixes this test case:

new-module-imports › valid › for (let i = 0; i < 10; i++) { }

    TypeError: pp.body.some is not a function
    Occurred while linting <input>:1

      162 |   }
      163 |
    > 164 |   return pp.body.some(n => {
          |                  ^
      165 |     function containsInitSpecifier(specifiers) {
      166 |       return specifiers.some(s => {
      167 |         return s.local.name === name;

      at some (lib/utils/new-module.js:164:18)
      at isInitImportedFrom (lib/rules/new-module-imports.js:59:44)

Same issue as in #459. Caused by #450.

@dcyriller the null check does not seem like a great fix, even though it solves the immediate problem. Can you please investigate if there is a better fix for this?

Fixes this test case:

```
new-module-imports › valid › for (let i = 0; i < 10; i++) { }

    TypeError: pp.body.some is not a function
    Occurred while linting <input>:1

      162 |   }
      163 |
    > 164 |   return pp.body.some(n => {
          |                  ^
      165 |     function containsInitSpecifier(specifiers) {
      166 |       return specifiers.some(s => {
      167 |         return s.local.name === name;

      at some (lib/utils/new-module.js:164:18)
      at isInitImportedFrom (lib/rules/new-module-imports.js:59:44)
```
@dcyriller
Copy link
Contributor

@bmish I've a submitted a PR to check against node types. It should be a better fix to these issues.

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

Successfully merging this pull request may close these issues.

2 participants