Eslint-plugin-import: `newline-after-import` breaks

[ljharb]

Cannot read property 'type' of undefined
TypeError: Cannot read property 'type' of undefined
    at getScopeBody ($PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:30:11)
    at $PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:104:27
    at Array.forEach (native)
    at $PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:103:22
    at Array.forEach (native)
    at EventEmitter.ProgramExit ($PWD/node_modules/eslint-plugin-import/lib/rules/newline-after-import.js:99:14)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.leaveNode ($PWD/node_modules/eslint/lib/util/node-event-generator.js:49:22)
    at CodePathAnalyzer.leaveNode ($PWD/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:627:23)
    at CommentEventGenerator.leaveNode ($PWD/node_modules/eslint/lib/util/comment-event-generator.js:110:23)

No idea what file it breaks on, but it started immediately after enabling this rule.

[benmosher]

Sounds like somehow you've got a scope that isn't a BlockStatement.

I tried to conjure such a line. Couldn't figure it out.

Without a reproduction, I'm not sure how to proceed.

[jfmengels]

Have trouble reproducing it too :confused:

[ljharb]

It would be awesome if there was a way for the current file path to be part of the stack trace :-/

Could the code here perhaps just check if it's a BlockStatement, and explicitly throw, with the filename, if it's not, rather than relying on the assumption that that's what will be there?

[jfmengels]

That sounds like something that would be better integrated directly in ESLint.

To debug your use-case, you could do what you said in your local node_modules. That sounds a lot simpler to me than to release a new version for this. You can use context.getFilename() to get the name of the currently treated file (http://eslint.org/docs/developer-guide/working-with-rules#the-context-object).

If you could help us by doing that, that'd be great!

[sindresorhus]

Someone posted an issue about this on XO: https://github.com/sindresorhus/xo/issues/128

[sindresorhus]

It would be awesome if there was a way for the current file path to be part of the stack trace

@ljharb 👍 That should definitely be added to ESLint. Can you open an issue there? It's indeed pretty annoying debugging broken rules when you have no idea where the error is coming from.

[ljharb]

@jfmengels the place where the error happens doesn't actually have context available - so in fact I do think releasing a new version that allows for better debugging is worth it, especially when the original rule had assumptions that weren't being unit tested.

That said, I'll try to play with the code in my local node_modules and see what I can come up with.

[ljharb]

k, I've figured it out - there's a switch statement in our code that looks like this:

  switch (renderData.modalViewKey) {
      case 'a':
        return require('../path/to/template.hbs')(renderData, options);
      case 'b':
        return require('../path/to/template.hbs')(renderData, options);
      case 'c':
        return require('../path/to/template.hbs')(renderData, options);
      case 'd':
        return require('../path/to/template.hbs')(renderData, options);
      case 'e':
        return require('../path/to/template.hbs')(renderData, options);
      case 'f':
        return require('../path/to/template.hbs')(renderData, options);
      case 'g':
        return require('../path/to/template.hbs')(renderData, options);
      case 'h':
        return require('../path/to/template.hbs')(renderData, options);
      case 'i':
        return require('../path/to/template.hbs')(renderData, options);
      case 'j':
        return require('../path/to/template.hbs')(renderData, options);
      case 'k':
        return require('../path/to/template.hbs')(renderData, options);
      case SOME_CONSTANT:
        return renderData.mainModalContent.clone();
      default:
        return renderData.mainModalContent.clone();
    }

When I comment this out, it doesn't error out.

[jfmengels]

Made a fix in https://github.com/benmosher/eslint-plugin-import/pull/395.