Open mkszepp opened 2 months ago
these are all expected
string | null
unless using separate types for read vs create vs edit. This is especially true for a findAll
where you are not guaranteed to get only remotely available records in the response. This means you need to ensure its just a string before invoking findRecord.constructor
being untyped is unsurprising, you'd need to cast.
- the correct type for includes is array, that string works has been a (happy) accident. I'll leave this ticket open because it should generate the proper include string from an array, and if it is not I'd like to know more about the setup to either confirm its an issue with just your app or if we have something we should fix.
Yes, when array will be joined to a string its okay for me. Wow, this means that code examples in repo docs are not correct or? For example: https://github.com/emberjs/data/blob/4badefdcbbdcb29e4a78645b780031f0372ad0a1/packages/store/src/-private/store-service.ts#L1710
Also this docs are saying that we should pass as string https://guides.emberjs.com/release/models/relationships/#toc_retrieving-related-records
- the correct type for id is
string | null
unless using separate types for read vs create vs edit. This is especially true for afindAll
where you are not guaranteed to get only remotely available records in the response. This means you need to ensure its just a string before invoking findRecord.
Okay thanks
- TypeScript does not allow typing constructors, so
constructor
being untyped is unsurprising, you'd need to cast.
Thanks for info
We do support string (we test, don't error for it), I guess I should just clarify that's not the intended way to use it and it was sort of an accident that it became possible (and got documented). Almost all of the code related to it treats it as an array which just happens to work because string and array APIs share a lot of overlap. We did eventually special case some handling of it as a string, typically to split it back into an array.
For typescript, we are only going to type the array story, as the string based story is both largely un-typeable and makes a lot of the features around include not work as seamlessly.
Also this docs are saying that we should pass as string
unfortunately the guides are a separate project the data team doesn't directly maintain, though we have been working to clean them up. @Baltazore has been doing work in there recently and could probably fix that quickly.
Thank you for the detailed explanation. It would also be good to get a codemode for the migration, because that would help everyone to correct existing code in project (whether TS or JS).
Maybe the team behind lint rules, should also provide a lint rule for it, so that one day you can get rid of wrong code and support only string[]
I have recently updated in our app the native type packages from
5.4.0-alpha.49
to5.4.0-alpha.91
After updating we have seen that there already fixed other incorrect types (thanks for that), but other types are now incorrect or maybe other improvements are bringing now incorrect TS errors
The option
include
was defined asarray
. it should be astring
, otherwise the JSONAPI call is incorrect. Passing as array generates url&include[]=pets
, which is not correct. demo - JSON:API Specs. This error is infindAll
,findRecord
,query
... (i think in all legacy functions)When we a
findAll
returns a list of models and we take for example the first to make additional calls likefindRecord
, we see errorError Argument of type 'string | null' is not assignable to parameter of type 'string | number'. Type 'null' is not assignable to type 'string | number'.ts(2345)
demo Expected:id
should bestring | number
, instead ofstring | null
We have used in some models
this.constructor.relationshipsByName;
. In this case we get error "Property 'relationshipsByName' does not exist on type 'Function'.". This works in JS... but typing looks like incorrect or missing seeAll TS errors you can find in this repo: https://github.com/mkszepp/ember-data-ts-errors