-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 strict null checks to @pixi/ticker #10087
base: v7.x
Are you sure you want to change the base?
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1341ae4:
|
const head = this._head; | ||
|
||
if (!head) | ||
{ | ||
return; | ||
} |
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.
Maybe it make sense to check this.head, and only after create a variable ?
const head = this._head; | |
if (!head) | |
{ | |
return; | |
} | |
if (!this._head) | |
{ | |
return; | |
} | |
const head = this._head; |
expect(appp.ticker).toBeInstanceOf(Ticker); | ||
expect(appp.ticker.started).toBe(false); | ||
expect(appp.ticker?.started).toBe(false); |
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.
Sorry, maybe I don't get this part, but it feels this 2 lines have conflicting logic, no ?
Description of change
Ref #8852
I have made the following adjustments to resolve the strict null check errors occurring under the packages/ticker directory.
packages/ticker/src/TickerPlugin.ts
_ticker
andticker
nullable and added optional chaining when referenced.packages/ticker/src/TickerListener.ts
TickerCallback
within this file.emit
method, to enable type inference for thefn
property, set the type of TickerCallback to a union type.T
of the classTickerListener
fromany
toRecord<string, any> | null
.fn
,context
,priority
, andonce
with the constructor's parameter definitions. This prevents redundant type management between the properties and constructor parameters. Additionally, sincenull
is set tofn
andcontext
within the destroy method, made them nullable.packages/ticker/src/Ticker.ts
null
is set to_head
and_requestId
within the destroy method, made them nullable._head
.Pre-Merge Checklist
npm run lint
)npm run test
)