Improve a lot of typing for BaseResource.
Review Request #13847 — Created May 8, 2024 and submitted — Latest diff uploaded
The BaseResource implementation in TypeScript had (and continues to
have) a number of issues. A lot of this stems from just bad design
decisions that were made before we even thought too much about typing or
polymorphism, but some of it is just mistakes or poor understanding of
TypeScript when I first converted it.This change does the following:
- Switches the order of
TOptions
andTExtraOptions
. I had followed
whatBackbone.Model
did, but didn't realize that the spina
BaseModel
swaps them. This was making it so our call to
CombinedModelConstructorOptions
was creating errors.- Adds a base resource data interface, and extends that for subclasses
that currently define the resource data structure.- Moved
payloadFileKeys
into the BaseResource implementation. This
was currently only defined in FileAttachment, but it's used inside
of BaseResource and by other (not-yet-typescriptified) subclasses.- Changed
validate
to take in aPartial<TDefaults>
instead of
TDefaults
, which matches our usage of this method.- Added type labels for everything that's in
prototypeAttrs
as
instance members too. This shuts up the type checker about these
magical properties.- Properly passed in the attributes type for
DraftReview
.- Added some missing type labels.
Changes 1 & 2 are technically breaking, but the only class that was
affected wasReviewRequest
, and it seems highly unlikely that any
third-party code is using this yet, so I want to do it now.What this doesn't fix is our decision to override
fetch
,save
, and
destroy
with incompatible signatures. I'm still puzzling over what to
do there to make it so we don't get huge horrible warnings about any
resource class not being compatible withBackbone.Model
.
Ran js-tests.