CreateAPI icon indicating copy to clipboard operation
CreateAPI copied to clipboard

Add official way to opt out of using Get

Open liamnichols opened this issue 2 years ago • 6 comments

Background

  • https://github.com/CreateAPI/CreateAPI/pull/82#discussion_r936948277

As it stands, to opt out of using Get, you must do three things:

  1. Use --module mode
  2. Update paths.imports to be an empty array
  3. Provide your own Request shim

While we believe that using CreateAPI with Get it the quickest and easiest option, we understand that not everybody is in a position to adopt Get. As a result, we need to provide a better way to disable use of the dependency.

Description

I can think of three scenarios:

  1. You want to use Get and you are happy with the default behaviour
  2. Your networking client already defines it's own Request in a module that should be imported
  3. You will build your networking client on top of the generated code and want a Request type generated for you

I did at first think that we should just generate Request if you opted not to use Get, but this will put some people who already import a different module defining Request in a situation where they can't use the generated code. It might be that like Get, the Request type is used for manually crafted requests too so defining it inside the CreateAPI generated package might not be appropriate.

Not sure how best to handle this...

liamnichols avatar Aug 04 '22 09:08 liamnichols

To support all three scenarios, I was thinking that maybe we could do it via either an enum or two bools:

Default Behaviour (using Get)

paths:
  requestType: useGet
paths:
  isUsingGet: true
  isGeneratingRequestType: false

Use Different Request

paths: 
  requestType: none
paths:
  isUsingGet: false
  isGeneratingRequestType: false
  imports:
  - MyAPIClient

Generate Request

paths:
  requestType: generate
paths:
  isUsingGet: false
  isGeneratingRequestType: true

Being honest, I'm not a fan of either of these approaches. And in fact, I'm wondering if we can combine this with improvements to how imports work since we already know we need some improvements in this area.

Currently, imports is just an array of strings. But it would be powerful if imports could also describe package dependencies (i.e the source and the version requirement) so that we can generate the complete Package definition. Going one step further, we could also declare what types (or features?) package provides to infer if we need to figure it out ourself...

For example:

dependencies:
- module: CoreLocation
  in: entities # or [entities, paths]
- module: MyAPIKit
  url: https://github.com/MyName/MyAPIKit.git
  branch: main
  provides:
  - Request

This would be a powerful feature that can enable a lot:

  1. We can infer if Request needs to be generated based on the presence of a provides + if the user opted out of Get.
  2. We could also apply it to other default dependencies? Although I'm not sure if they make so much sense as they're all very small.
  3. We could apply it to any type (i.e generated types) in theory.
  4. It makes --package generation more powerful because you can define your own dependencies to be included
  5. We can relatively easily transition from the old approach since it also works with system imports/imports that aren't tied to SPM.

So back to the original examples

Default Behaviour (using Get)

# No configuration needed

Use Different Request

dependencies:
- module: MyAPIKIt
  url: https://github.com/MyName/MyAPIKit.git
  from: 1.0.0
  provides:
  - Request

Generate Request

paths:
  isGeneratingRequestType: true

The logic for importing Get would then be something like this:

var requiredDependencies: [RequiredDependency] = []
var customDependencies: [CustomDependency] = // ...
if !customDependencies.contains(where: { $0.provides.includes("Request") }) && !options.paths.isGeneratingRequest {
    requiredDependencies.append(RequiredDependency(
        module: "Get",
        importIn: [.paths],
        url: "https://github.com/kean/Get.git",
        from: "1.0.2"
    ))
}

I think this could work... I'll open a separate ticket for it 😄 Feels like a 0.2.0 thing though

liamnichols avatar Aug 04 '22 09:08 liamnichols

From my comment in #85, we can create a paths.requestProvider: Get || custom option. With paths.requestProvider: Get, all we do is simply use Get as an import in Paths and add it as an import to the files.

For paths.requestProvider: custom, users have two options:

  • create a custom Request type locally
  • use a Request type from a package. Using https://github.com/CreateAPI/CreateAPI/pull/135 and paths.imports, this is quite easy.

LePips avatar Aug 11 '22 18:08 LePips

There is still the scenario to cover where we want to generate the Request type as part of the generated output.

This is particularly useful when using create-api to generate the sources directly into an existing target where the api client lives side by side.

liamnichols avatar Aug 11 '22 19:08 liamnichols

I would assume in that scenario that the Request type is a custom local type, so the user would still have to provide the Request type.

Or, we can expand the options to paths.requestProvider: Get || local || external.

  • Get, as it is now
  • local, generates a boilerplate Request type
  • external, doesn't generate anything. Request should come from some import or is defined elsewhere, like alongside an api client in the same module.

Edit: actually, since I just glanced at Get.Request, we should just have paths.requestProvider: local || external with the same reasonings. Get would have to be imported manually so that devs can ultimately choose what their networking interface is.

Or now that it's a binary option, we have the flag generateRequestType: true || false

LePips avatar Aug 11 '22 19:08 LePips

Please see the bottom of the following comment first: https://github.com/CreateAPI/CreateAPI/pull/136#issuecomment-1212391259


The question regarding this specifically is: should we tightly couple CreateAPI to a specific networking package? The meaning of tightly couple is the same as in the comment.

For this specifically, my opinion is strongly no. As a generator, we don't care about the underlying networking implementation by the developer. By generalizing the Request object, we just do that: create a request and the developer passes it onto the implemented networking stack, post-processing if desired to match their stack.

As said in the attached comment, we can happily recommend Get and provide the boilerplate config options in documentation, probably when talking about how to use CreateAPI and describing the Request type. So, the generateRequestType option seems the most optimal in my opinion.

LePips avatar Aug 12 '22 01:08 LePips

An enum sounds good. Maybe something like this:

apiClient

Type: Enum
Default: get

The networking library to use:

  • get – a thin wrapper on top of URLSession (? maybe name it urlsession)
  • alamofire – (?) generates Request and extensions to Alamofire that work with it
  • moya – (?) add TargetType conformance to Request
  • vapor – (?) generates code for backend-to-backend communication
  • custom – generates Request type and the user is expected to either use it directly or provide adapters for it

CreateAPI and Get are part of the same "ecosystem". It can be considered the same as using URLSession directly.

If get is used, it would be nice to also generate a few typealiases for its types so that the user won't have to think/import it. For them, it'll look like a simple URLSession-based networking stack provided by the generated package.

typealias APIClient = Get.APIClient

For more advanced use-cases (e.g. no Request type), I think CreateAPI should support templates, (Stencil?). Not even going to suggest it now though, which OK I just did :)

kean avatar Aug 12 '22 04:08 kean