yelp / dataloader-codegen Goto Github PK

Currently when a batch resource is configured, if the batchKey for the underlying endpoint is not a required param, there will be flow type check errors raised.

Issue sample with details

For an endpoint like:

    TestUserApi:
        getUsers:
            isBatchResource: true
            docsLink: http://hostname/?service=testservice#/testuserapi/get_users
            batchKey: user_ids
            newKey: user_id
            reorderResultsByKey: id

where the user_ids param is non-required, the generated clientlib would look like:

getUsers($ReadOnly<{| user_ids?: $ReadOnlyArray<number>, user_emails?: $ReadOnlyArray<string>, |}>, options?: RequestOptions): Promise<Array<User>>,

in the lib/api.js.flow file. Note for user_ids it looks like: user_ids?: $ReadOnlyArray<number>.

At runtime, everything works as expected, as long as the user_ids is supplied properly.

But when doing flow type validation, it will cause below error:

Cannot instantiate `$ElementType` because an index signature declaring the expected key / value type is missing in
undefined [1].

                        v------------
               user_id: $ElementType<
                 $PropertyType<
                   $Call<
                     ExtractArg,
                     [
                       $PropertyType<
                         $PropertyType<ResourcesType, "TestUserApi">,
                         "getUsers"
                       >
                     ]
                   >,
                   "user_ids"
                 >,
                 0
               >
                ^
References:
   node_modules/@test-clientlib/testapi/lib/api.js.flow:
        getUsers($ReadOnly<{| user_ids?: $ReadOnlyArray<number>, user_emails?: $ReadOnlyArray<string>,  |}>, options?: RequestOptions): Promise<Array<User>>,
                                         ^^^^^^^^^^^^^^^^^^^^^ [1]

In Yelp/bravado, developers can specify per-request configuration by passing in _request_options to the service call. A potential usecase is specifying headers which are consumed by the downstream service (or services in the middle of the request path), such as Yelp/casper.

The python api looks like:

client = SwaggerClient.from_url(...)
request_options = { ... }
client.pet.getPetById(petId=42, _request_options=request_options).response().result

A potential solution is to expose a similarly named parameter in the JS equivalents.

This feature request is to support endpoints with multiple batch ids.

Example use case

The endpoint that would benefit from this would be an endpoint that takes multiple batch ids and returns a nested object, such as an endpoint with the following definition:

Endpoint request
business_ids: str[]
features: str[]

Endpoint response

{
  "businessId1": {
    "feature1": "yes",
    "feature2": "no"
  },
  "businessId2": {
    "feature1": "yes"
  }
}

With the current dataloaders functionality, we would be able to set:

batchKey: business_ids
newKey: business_id

which would group multiple requests for that single feature for different businesses, but not multiple requests for different features on the same business. Where there is a large number of features, this could get expensive pretty quickly.

Proposed New Functionality

The ability to set multiple (or at least a secondary level of) batch keys, and then where right now we get back response[business_id] from the dataloader, we would get back something like response[business_id][feature] as the value at that feature for that business.

batchKey: business_ids
newKey: business_id
secondaryBatchKey: features
secondaryNewKey: feature

Right now, we assume that resources look something like this:

function getFoo({ count, type, somethingElse }, fetch, options) {
  ...

i.e. we assume that the underlying resource takes its actual data arguments in the first parameter as an object. The rest are meta arguments which have defaults, and we never really care about overriding them from inside a resolver method.

In this world, a dataloader interface would look like this:

getFoo.load({ count: 4, type: 'baz', somethingElse: 'qux' });

This fits nicely with the interface of dataloader anyway (you can only send one argument as a key)

This assumption has been made because that's the interface of the underlying resources Yelp currently uses. But this might not always be true!

Proposed New Functionality

We should add an option - something like "useAllArguments" so if you wanted to override fetch from a resolver method, your dataloader interface would look like this:

getFoo.load([{ count: 4, type: 'baz', somethingElse: 'qux' }, myCustomFetch]);

What's the workaround in the meantime

You can use resource middleware to transform the thing that actually gets send to the underlying resource for now, to add extra arguments

Opening an issue to track adding native TypeScript support for output of this library.

(Until we have a native option for TypeScript, consider using https://github.com/Khan/flow-to-ts or https://github.com/bcherny/flow-to-typescript ?)

Would you be interested in TypeScript for this library? Let us know in this thread!

Internal ticket: CLIENTDATA-511

e.g. an endpoint might be the following shape:

GET /widgets?ids=1,2,3,6,8

returns

{
	"1": {
		"widgetName": "foo"
	},
	"2": {
		"widgetName": "bar"
	},
	"5": {
		"widgetName": "baz"
	},
	"8": {
		"widgetName": "qux"
	}
}

We currently don't support this as a batch endpoint - we've seen this in the wild, and should support it :)

This looks super nifty https://github.com/atlassian/better-ajv-errors and a way to improve the DX

(we use ajv for parsing the dataloader-codegen config file)