Change ResourceHandler from interface to base class #5574
Conversation
|
@slimbuck I recall you saying previously that you don't like the asset system/resource handlers/etc, so I'm interested to get your reaction to this. |
| @@ -28,36 +54,32 @@ class ResourceHandler { | |||
| * ResourceLoader. | |||
| */ | |||
| load(url, callback, asset) { | |||
| throw new Error('not implemented'); | |||
| // do nothing | |||
There was a problem hiding this comment.
I think this default implementation should invoke callback with failed result. Any caller hitting this function would hang indefinitely (and existing handler implementations that do nothing are probably wrong).
There was a problem hiding this comment.
Any conclusion on this? If handlers that do nothing are probably wrong, I guess we can just keep the throw statement aswell?
|
Niiice, I much prefer a proper prototype chain and the reduced amount of "special" stuff to care about in Make sure to only replace |
|
@kungfooman Yes, I realized yesterday I need to add |
|
Going to remove #6043 since it is a duplicate PR @willeastcott I have updated the branch by merging main in. |
Yebo! 👍🏻 |
This PR is a reasonably simple refactor of the
ResourceHandlers. Specifically, it switches the effectively unused 'interface'ResourceHandler(despite JavaScript not supporting interfaces) into a base class.I confirm I have read the contributing guidelines and signed the Contributor License Agreement.