refactor: inline AppBase tick as a class-field arrow#8639
Conversation
Replace the module-level `makeTick` closure factory with a class-field arrow function `tick` on AppBase. Removes the `no-use-before-define` workaround in `init()`, the `const application = _app` alias, and the now-unused `MakeTickCallback` typedef. No behaviour change: `tick` remains a writable instance property, so subclasses (e.g. the editor's ViewportApplication) can still reassign it after `super()`. Made-with: Cursor
The assignment was a pair to the old `this.tick = makeTick(this)` in init(). Now that `tick` is a permanent class-field arrow, nulling it is both asymmetric with how it is created and a type error against its declared signature. The arrow's own `if (!this.graphicsDevice) return;` guard already provides graceful no-op behaviour post-destroy, and the in-flight rAF is cancelled by `AppBase.cancelTick(this)`. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Refactors AppBase’s main loop scheduling by replacing the module-scope makeTick closure factory with an auto-bound class-field arrow function tick, simplifying initialization and linting while keeping tick as an internal, writable instance property.
Changes:
- Introduces
tick = (timestamp, xrFrame) => { ... }as a class-field arrow function onAppBase. - Removes the
makeTickfactory and its associated@callback MakeTickCallbacktypedef. - Removes the
this.tick = makeTick(this)assignment frominit().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Is this a breaking change? |
I mean, technically, yes because that symbol is now gone from the API reference. But who would be using that? |
|
perhaps leave the old function, and add Debug.removed to it in case somebody used it. |
It's not a function. It's a callback type. But I honestly can't think of how it can be used by users. I think it leaked by mistake. |
Summary
makeTickclosure factory inAppBasewith a class-field arrow functiontick, removing theno-use-before-defineworkaround and theconst application = _appalias.@callback MakeTickCallbackJSDoc typedef.this.tick = makeTick(this)assignment (and its comment) frominit().Rationale
The old pattern was a forward-declared module-scope factory called from
init(), which required an ESLint disable comment on every tick of development and obscuredthisbehind a capturedapplicationalias. A class-field arrow is:this.tickcan still be passed straight torequestAnimationFrame,this.tickavailable to subclass constructor bodies.Public API
tickis@ignore(internal), and remains a writable instance property, so subclasses like the editor'sViewportApplicationcan continue to reassign it aftersuper().Test plan
npx eslint src/framework/app-base.js— clean.