fix: check exports exist before access to it #2990
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2990 +/- ##
=======================================
Coverage 81.81% 81.81%
=======================================
Files 77 77
Lines 3795 3795
Branches 1257 1260 +3
=======================================
Hits 3105 3105
Misses 690 690 ☔ View full report in Codecov by Sentry. |
| @@ -534,15 +534,15 @@ module.exports = { | |||
| } | |||
|
|
|||
| // special case: export * from | |||
| const exportAll = exports.get(EXPORT_ALL_DECLARATION); | |||
| const exportAll = exports && exports.get(EXPORT_ALL_DECLARATION); | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
I think we still need to report from line 567 to be shown:
context.report(
node,
`exported declaration '${value}' not used within other modules`,
);
At the end
There was a problem hiding this comment.
The error console says the file has no exports then why it should report any export error? Or that just means an internal bug?
There was a problem hiding this comment.
The scenario is following:
- create new file and define export it in
- see eslint error that export is not used anywhere.
Without this PR we see eslint crash and no longer provide other checks and autofixes.
I assume this is internal bug mentioned in console warning, file has export, so this should be fixed somehow. till then this fix works as well
Without PR eslint process crashes |
|
I understand the repro, but we need a code regression test. |
|
Is there an example in this repo of regression test with dynamically added code to mirror for this case? |
|
Not that I know of. Unfortunately this is a really difficult failure case to reproduce in tests. |
while this issue exist #2866, this fix helps to prevent eslint from crashing.