balena-io / balena-sdk

The SDK to make balena powered JavaScript applications.
https://www.balena.io/
Apache License 2.0
145 stars 46 forks source link

Type error when adding an already expanded ReverseNavigationResource in the $select list #1342

Open barribarrier opened 1 year ago

barribarrier commented 1 year ago

Expected Behavior

No type error

Actual Behavior

With strictNullChecks set to true, selecting 'device_tag' field results in type error. This is not specific to only 'device_tag' field. Any field that has a type ReverseNavigationResource<T> fails.

Type '"device_tag"' is not assignable to type '"id" | "actor" | "created_at" | "modified_at" | "custom_latitude" | "custom_longitude" | "device_name" | "download_progress" | "ip_address" | "public_address" | "mac_address" | ... 43 more ... | "should_be_managed_by__supervisor_release"'. ts(2322)

With strictNullChecks set to false, there's no error.

Steps to Reproduce the Problem

1.

git clone https://github.com/barribarrier/strictNullChecks-balena-sdk-issue.git
cd strictNullChecks-balena-sdk-issue
npm install
npm run build

OR

  1. tsconfig.json

    {
    "compilerOptions": {
    "strictNullChecks": true
    }
    }
  2. index.ts

    
    import { getSdk } from 'balena-sdk';

const balena = getSdk();

balena.models.device.getAllByApplication( '123456', { $select: ['uuid', 'device_tag', 'device_environment_variable', 'created_at'], $expand: { device_tag: { $select: ['tag_key', 'value'], }, }, }, );

3. Run
```bash
tsc

Specifications

References

thgreasi commented 1 year ago

Hi, Thanks for raising this and for the POC repo. I think this is actually the intended behavior, since ReverseNavigationResource properties in $select have no effect and they normally are supposed to be provided in $expands. The query that you provided above will actually throw a 500 error when running against balenaCloud, because of having the device_environment_variable ReverseNavigationResource in the $select.

The part that this becomes a bit weird though is that having the device_tag in the $select happens to be fine by the backend as long as it is also part of an $expand. The SDK's typings are trying to be slightly stricter on this and block ReverseNavigationResource properties in $selects completely (rather than checking whether they also show up in the $expand) to protect the user in the common case. The typings are explicitly handling this case here: https://github.com/balena-io/balena-sdk/blob/8bc6c447aea2b5589a322cfeb322ee4a731714b4/typings/pinejs-client-core.d.ts#L53

Let us know your thoughts on this.

barribarrier commented 1 year ago

Oh, I didn't know that. I just assumed specifying them in $select was mandatory. Thanks.