Add support for ES6 bare imports #468
Conversation
| // Named imports & namespace imports handled by other walker methods. | ||
| // importClause will be null for bare imports. | ||
| if (importClause != null && importClause.name != null) { | ||
| const variableIdentifier = importClause.name; |
There was a problem hiding this comment.
it's inappropriate to use const for these variables. we have no way of enforcing these -- someone can still do variableIdentifier.setRandomValue(123) and TypeScript wouldn't know. we should be reserving const for literal values only.
There was a problem hiding this comment.
Doesn't matter though. I'm strongly in favor of using the strictest specifier whenever possible for variables. It signals to the developer that the variable binding should never change, and devs are expected to know JS/TS semantics enough to know that the values inside that object might change.
There was a problem hiding this comment.
Also, that's unrelated to this change. Bring it up in a separate issue/PR.
|
Review status: 0 of 5 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful. src/rules/noUnusedVariableRule.ts, line 59 [r1] (raw file): src/rules/noUseBeforeDeclareRule.ts, line 50 [r1] (raw file): Comments from the review on Reviewable.io |
| @@ -56,8 +56,9 @@ class NoUnusedVariablesWalker extends Lint.RuleWalker { | |||
| if (!Lint.hasModifier(node.modifiers, ts.SyntaxKind.ExportKeyword)) { | |||
| const importClause = node.importClause; | |||
|
|
|||
| // named imports & namespace imports handled by other walker methods | |||
| if (importClause.name != null) { | |||
| // Named imports & namespace imports handled by other walker methods. | |||
There was a problem hiding this comment.
fixed all code comments in #470. revert this comment to its original format. same thing in noUseBeforeDeclareRule.
Fixes #451 This affects the following rules: - `no-unused-variable` - `no-use-before-declare` - `whitespace` (it was already handled here, but I added a regression test)
21ad3d5 to
5398f44
Compare
|
addressed feedback and --amended |
|
👍 waiting for builds to finish |
|
why do builds take so long |
|
TravisCI has been pretty slow lately. 🐌 |
Add support for ES6 bare imports
Fixes #451
This affects the following rules:
no-unused-variableno-use-before-declarewhitespace(it was already handled here, but I added a regression test)