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

Handle ignored files when re-exporting from file #348

Closed
sohkai opened this issue May 18, 2016 · 2 comments
Closed

Handle ignored files when re-exporting from file #348

sohkai opened this issue May 18, 2016 · 2 comments
Assignees
Labels
Milestone

Comments

@sohkai
Copy link

sohkai commented May 18, 2016

Currently, ignored files return null when you try to get an import from them (see getExports#L79). However, this causes the import/export rule to throw an exception when you try to re-export everything from a file who is re-exporting from an ignored file, because ExportMap's forEach expects each of its re-exports to be able to get their imports (see getExports#L331).

For example, running ESLint with import/exports turned on throws on these files:

// test.js
export { default as x } from 'your-package'; // your-package is in the ignored settings (eg. in node_modules)

// reexport.js
export * from './test';

Exception:

Cannot read property 'get' of null
TypeError: Cannot read property 'get' of null
    at .../node_modules/eslint-plugin-import/lib/core/getExports.js:422:48
    at Map.forEach (native)
    at ExportMap.forEach (.../node_modules/eslint-plugin-import/lib/core/getExports.js:419:20)
    at EventEmitter.ExportAllDeclaration (.../node_modules/eslint-plugin-import/lib/rules/export.js:82:21)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.enterNode (.../node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode (.../node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:607:23)
    at CommentEventGenerator.enterNode (.../node_modules/eslint/lib/util/comment-event-generator.js:97:23)
    at Controller.traverser.traverse.enter (.../node_modules/eslint/lib/eslint.js:886:36)

It seems like the simplest thing to fix this would be to just check that getImport() exists before trying to use .get() on it (although I'm not really sure what the first argument to the callback represents -- would null as a possibility make sense there?), but from a cursory glance, I also feel that using null to represent ignored files seems somewhat brittle?

@benmosher benmosher self-assigned this May 19, 2016
@benmosher benmosher added the bug label May 19, 2016
@igrayson
Copy link

igrayson commented Jun 17, 2016

I came across something that manifested slightly differently, but sounds related. Let me know, and I can open a new issue.

Symptom:

Cannot read property 'size' of null
TypeError: Cannot read property 'size' of null
    at /Users/ian/workspace/eslint-export-all/node_modules/eslint-plugin-import/lib/core/getExports.js:463:29
    at Map.forEach (native)
    at ExportMap.get (/Users/ian/workspace/eslint-export-all/node_modules/eslint-plugin-import/lib/core/getExports.js:462:25)
    at processBodyStatement (/Users/ian/workspace/eslint-export-all/node_modules/eslint-plugin-import/lib/rules/namespace.js:66:27)
    at Array.forEach (native)
    at EventEmitter.Program (/Users/ian/workspace/eslint-export-all/node_modules/eslint-plugin-import/lib/rules/namespace.js:84:12)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:188:7)
    at NodeEventGenerator.enterNode (/Users/ian/workspace/eslint-export-all/node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode (/Users/ian/workspace/eslint-export-all/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:607:23)

This happens when applying the import/namespace rule on:

test.js:

import * as foo from './foo';

foo.js:

export * from './bar';

bar.js:

module.exports = { };
  • bar.js must be ignored.
  • bar.js must use module.exports syntax.

It could be that hasExports is failing to detect the fact that module.exports is a valid export statement; or that this apparatus as a whole is not prepared to deal with the old-fashioned module.exports style?

Test repo to reproduce: https://github.com/igrayson/eslint-export-all
npm install && npm test.

Note: this failure case is basically guaranteed to trigger when using export * from 'some-node-module' from linted application code.

@benmosher
Copy link
Member

@igrayson: sounds like the same issue.

FWIW, it won't recognize module.exports as a namespace, but it definitely shouldn't throw an exception.

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

No branches or pull requests

3 participants