-
Notifications
You must be signed in to change notification settings - Fork 3k
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
style(linting): enable ' --noUnusedLocals' option #2104
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import { Observable } from './Observable'; | ||
import { Subscriber } from './Subscriber'; | ||
import { TeardownLogic } from './Subscription'; | ||
|
||
export interface Operator<T, R> { | ||
call(subscriber: Subscriber<R>, source: any): TeardownLogic; | ||
call(subscriber: Subscriber<R>, source: Observable<T>): TeardownLogic; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,8 @@ export class ArrayLikeObservable<T> extends Observable<T> { | |
|
||
protected _subscribe(subscriber: Subscriber<T>): TeardownLogic { | ||
let index = 0; | ||
const { arrayLike, scheduler } = this; | ||
const arrayLike = this.arrayLike; | ||
const scheduler = this.scheduler; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason? I think we want to keep our existing, destructure style. Is there a subtle difference I'm missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for to avoid a TypeScript compiler's bug (It will be fixed in TS2.1). |
||
const length = arrayLike.length; | ||
|
||
if (scheduler) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,7 +104,7 @@ class DelaySubscriber<T> extends Subscriber<T> { | |
})); | ||
} | ||
|
||
private scheduleNotification(notification: Notification<any>): void { | ||
private scheduleNotification(notification: Notification<T>): void { | ||
if (this.errored === true) { | ||
return; | ||
} | ||
|
@@ -134,7 +134,7 @@ class DelaySubscriber<T> extends Subscriber<T> { | |
} | ||
|
||
class DelayMessage<T> { | ||
constructor(private time: number, | ||
private notification: any) { | ||
constructor(public time: number, | ||
public notification: Notification<T>) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did these need to change from private to public? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old code was wrong. These members should be public to be used in |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,7 @@ class LastSubscriber<T, R> extends Subscriber<T> { | |
constructor(destination: Subscriber<R>, | ||
private predicate?: (value: T, index: number, source: Observable<T>) => boolean, | ||
private resultSelector?: ((value: T, index: number) => R) | void, | ||
private defaultValue?: any, | ||
defaultValue?: any, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did this need to be changed to not be a private property? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this option make it the error. |
||
private source?: Observable<T>) { | ||
super(destination); | ||
if (typeof defaultValue !== 'undefined') { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these annotations necessary? I figured TS could easily infer the types from the assignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't make it, TypeScript compiler make it error (This might be a compiler's issue?). But there is no problem. We can avoid such error with just writing expliciltly code.