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 lint check for unused imports #1212

Closed
cramforce opened this issue Dec 21, 2015 · 3 comments · Fixed by #1220
Closed

Add lint check for unused imports #1212

cramforce opened this issue Dec 21, 2015 · 3 comments · Fixed by #1220
Assignees

Comments

@cramforce
Copy link
Member

Imports should be used or not done.

CC @erwinmombay

@erwinmombay
Copy link
Member

So we can't really check for unused imports since imports can have side effects but per the conversation in https://github.com/ampproject/amphtml/pull/1211/files, we can turn on unused reference lint checks but it currently errors out cause it also checks for unused properties and we have the following scattered all over our code base.

class SomeClass {
  constructor() {
    /** @private */
    this.someProp_;
  }
}

I'll have to do a cleanup of that first, but i 100% agree this should be on.

@erwinmombay erwinmombay self-assigned this Dec 21, 2015
@cramforce
Copy link
Member Author

Yeah, we should really change those by adding = undefined.

I think that check would really give us robustness!

@erwinmombay
Copy link
Member

i stand corrected, the one i stated was for unused expression. but where it actually errors out are on our typedefs, like so:

/**
 * @typedef {{
 *   newStackIndex: number
 * }}
 */
let ViewerHistoryPoppedEvent;

now we need to add either a Def suffix or a Interface suffix so that they are ignored by the linter. (for typdefs, templates and class interfaces)

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

Successfully merging a pull request may close this issue.

2 participants