Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect code generation: String instead of Int #23

Closed
nikola-mladenovic opened this issue Nov 18, 2016 · 27 comments
Closed

Incorrect code generation: String instead of Int #23

nikola-mladenovic opened this issue Nov 18, 2016 · 27 comments
Labels
codegen Issues related to or arising from code generation
Milestone

Comments

@nikola-mladenovic
Copy link

nikola-mladenovic commented Nov 18, 2016

Steps to reproduce:

The resulting API.swift that was generated using apollo-ios version 0.4 and apollo-codegen version 0.9 contains: public let centAmount: String (MoneyDetails struct property). The actual type from the schema:

{
"name": "centAmount",
"type": {
  "kind": "NON_NULL",
  "name": null,
  "ofType": {
    "kind": "SCALAR",
    "name": "Long",
    "ofType": null
  }
}
@martijnwalraven
Copy link
Contributor

According to the GraphQL spec, Long is not a built-in scalar, so it is treated as a custom scalar. Right now in Apollo iOS, all custom scalars are mapped to string (which is also how they are encoded in JSON).

I would definitely like to add better support for custom scalars, which should give you the opportunity to define your own mapping.

But in this case, do you really need Long here, or would Int suffice?

@martijnwalraven
Copy link
Contributor

thanks @martijnwalraven! judging by the nature of centAmount field, I think the value can be pretty high in some cases
the current workaround could be manually altering the local schema file; is there anything better you’d suggest?

I hope to add better support for custom scalars soon, but in the meantime there is a temporary apollo-codegen option --passthrough-custom-scalars that doesn't attempt to map custom scalars to string but keeps them as is. That means you're responsible for defining the type and making sure it is JSONDecodable and JSONEncodable.

I haven't tested it, but something like this should work:

typealias Long = Int64

extension Int64: JSONDecodable, JSONEncodable {
  public init(jsonValue value: JSONValue) throws {
    guard let string = value as? String else {
      throw JSONDecodingError.couldNotConvert(value: value, to: String.self)
    }
    guard let number = Int64(string) else {
      throw JSONDecodingError.couldNotConvert(value: value, to: Int64.self)
    }

    self = number
  }

  public var jsonValue: JSONValue {
    return String(self)
  }
}

@AnandWalvekar
Copy link
Contributor

Hi @martijnwalraven
I am getting an error on iOS client

Error
"Error at path "history.itemid": couldNotConvert(value: 1100025, to: Swift.String)"

System Info

  • apollo-codegen version : 0.16.5

  • Apollo Client SDK version : (0.6.2)

  • schema.json type
    { "defaultValue": null, "name": "itemId", "description": null, "type": { "kind": "NON_NULL", "name": null, "ofType": { "kind": "SCALAR", "name": "Long", "ofType": null } } }

What I tried
Tried extension you have mentioned and updated the build generation script in Xcode -> Build Phases -> Ran Script
$APOLLO_FRAMEWORK_PATH/check-and-run-apollo-codegen.sh generate $(find . -name '*.graphql') --schema schema.json --output API.swift --passthrough-custom-scalars

Please let me know if I am going wrong somewhere

@martijnwalraven
Copy link
Contributor

@AnandWalvekar: Does the generated API code reference Long? Where are you including typealias Long = Int64 and the extension?

@AnandWalvekar
Copy link
Contributor

@martijnwalraven
Yes it is having Long
public var itemId: Long
I have included both typealias & extension in a new file in my project as
import Apollo
public typealias Long = Int64

But still getting the same error

@martijnwalraven
Copy link
Contributor

@AnandWalvekar: That is surprising. Can you check if the generated API code includes a typealias Long = String by any chance? (It shouldn't, because you specify --passthrough-custom-scalars. But then even if it doesn't pick up your typealias Long, it should fail with a compiler error.)

@AnandWalvekar
Copy link
Contributor

@martijnwalraven
It works if I write like below
guard let long = value as? Long else { throw JSONDecodingError.couldNotConvert(value: value, to: String.self) } self = long

Is this the solution?

@martijnwalraven
Copy link
Contributor

martijnwalraven commented Sep 4, 2017

@AnandWalvekar: Ah, so you're encoding Long as a JSON number instead of a string! If that's the case, something like your code would work (although you may want to change the error message as well).

Note that numbers in JavaScript can't represent 64-bit integers however, so unless you're sure the value will never exceed Number.MAX_SAFE_INTEGER (2^53 - 1), that might lead to unexpected problems.

@AnandWalvekar
Copy link
Contributor

AnandWalvekar commented Sep 4, 2017

@martijnwalraven
Understood. Thank you very much for your quick support.

@NiltiakSivad
Copy link

NiltiakSivad commented Aug 20, 2018

@martijnwalraven suggestion worked fine for me originally, but the latest Apollo code generation script installation changes does not accept the --passthrough-custom-scalars option. Worked with check-and-run-apollo-codegen.sh (Apollo 0.8) but does not work with the new check-and-run-apollo-cli.sh (Apollo 0.9.2).

@szantogab
Copy link

@NiltiakSivad same problem here.. :(

@NiltiakSivad
Copy link

@szantogab I re-read martijnwalraven's post and he does mention it's a "temporary" flag. I'm not sure if anything is mentioned about its removal and future plans in another issue/merge request, I need to investigate more, if I find anything I'll post here.

@szantogab
Copy link

@NiltiakSivad I dug a little deeper, and I found that Apollo CLI now expects the parameter this way: --passthroughCustomScalars.

You can check out Apollo CLI's other parameters here:
https://github.com/apollographql/apollo-cli#apollo-codegengenerate-output

Hope this helps you :)

@daksharma
Copy link

daksharma commented Oct 16, 2018

Hi @martijnwalraven
It seems like the typealias was a viable solution for many people. I tried the typealias Long = Int64 and the extension that you have posted above.
When generating the API.swift file I am using the ----passthroughCustomScalars flag, when building the project Xcode/Swift complains about Property cannot be declared public because its type uses an internal type.

I am using Apollo through Cocoapods and the Apollo cli (not apollo-codegen) as well as the script provided in the installation process of the apollo ios website.
I tried both Swift 4 and Swift 4.2 in the build settings. but the error persists.

This error is in the API.swift file. No typealias is declared in the generated file.
screen shot 2018-10-16 at 1 20 16 pm

Since API.swift should not be modified, any idea on how this can be resolved.

EDIT: Solved :)

@jtoce
Copy link

jtoce commented Nov 6, 2018

Since API.swift should not be modified, any idea on how this can be resolved.

EDIT: Solved :)

I copy the code into Long.swift and then changed the line to read public typealias Long = Int64.

@john1452
Copy link

Hi @martijnwalraven

Is there a workaround to use "Any" as custom type?
One of the API i am consuming returns "Object" type which can contain Int Decimal or a string value.

I tried the following but it will not work.

public typealias Object = Any
extension Object : JSONDecodable, JSONEncodable { }

Any help is appreciated

Thanks!

@daoseng33
Copy link

daoseng33 commented Jan 16, 2019

Remember to import Apollo and conform "JSONDecodable", not inherit "JSONDecoder"...
(I'm stuck for this for an hour... such a fool)

@niraj1991
Copy link

niraj1991 commented Mar 28, 2019

Hi @martijnwalraven

I am getting an error on iOS client. Incorrect code generation: String instead of Array.

The resulting API.swift that was generated using apollo-ios

GraphQLField("reactiondata", type: .scalar(String.self)),

 public var reactiondata: String? {
        get {
          return resultMap["reactiondata"] as? String
        }
        set {
          resultMap.updateValue(newValue, forKey: "reactiondata")
        }
      }

The actual type from the schema file:

{ "name": "reactiondata", "description": null, "args": [], "type": { "kind": "SCALAR", "name": "Array", "ofType": null }, "isDeprecated": false, "deprecationReason": null },

If I am try with --passthroughCustomScalars Also I am geting error.

The resulting API.swift that was generated with --passthroughCustomScalars

public typealias Array = [AnyObject]

GraphQLField("reactiondata", type: .scalar(Array.self)),

public var reactiondata: Array? { get { return resultMap["reactiondata"] as? Array } set { resultMap.updateValue(newValue, forKey: "reactiondata") } }

I am geting following error.
Screenshot 2019-03-28 at 12 13 22 PM
Screenshot 2019-03-28 at 12 13 13 PM


`# Type a script or drag a script file from your workspace to insert its path.
APOLLO_FRAMEWORK_PATH="$(eval find $FRAMEWORK_SEARCH_PATHS -name "Apollo.framework" -maxdepth 1)"

if [ -z "$APOLLO_FRAMEWORK_PATH" ]; then
echo "error: Couldn't find Apollo.framework in FRAMEWORK_SEARCH_PATHS; make sure to add the framework to your project."
exit 1
fi

cd "${SRCROOT}/${TARGET_NAME}"
$APOLLO_FRAMEWORK_PATH/check-and-run-apollo-cli.sh codegen:generate --queries="$(find . -name '*.graphql')" --passthroughCustomScalars --schema=schema.json API.swift

Please let me know if I am going wrong somewhere

@martijnwalraven
Copy link
Contributor

@niraj1991 The type in the schema you posted seems to be a custom scalar called Array, not an actual GraphQL list type. If reactiondata is meant to be represented as an array, you should change the type in the schema.

@niraj1991
Copy link

@martijnwalraven Thanks for writing to me.
I am updating my above post. can you please check again?

@martijnwalraven
Copy link
Contributor

@niraj1991 You'll have to change reactiondata to be an actual list in your schema, not a scalar. You can't generate a scalar with the name Array and expect it to map to a Swift Array.

@niraj1991
Copy link

@martijnwalraven I changed the schema file like below.

{ "name": "details", "description": null, "args": [], "type": { "kind": "SCALAR", "name": "Json", "ofType": null }, "isDeprecated": false, "deprecationReason": null }

But I am Getting below error.

`Error at path "posts.0.reactiondata": couldNotConvert(value: <__NSArray0 0x600002408050>(
)
, to: Swift.Dictionary<Swift.String, Swift.Optional<Any>>)`

The resulting API.swift that was generated like below.

 public static let selections: [GraphQLSelection] = [
        GraphQLField("reactiondata", type: .scalar(Json.self)),
      ]
public var reactiondata: Json? {
        get {
          return resultMap["reactiondata"] as? Json
        }
        set {
          resultMap.updateValue(newValue, forKey: "reactiondata")
        }
      }

Also I am creating typealias like below.

public typealias Json = [String : Any?]

Let me know if you need any other details, I would be happy to share you.

Please let me know if I am going wrong somewhere.

Thanks in advance.

@martijnwalraven
Copy link
Contributor

@niraj1991 It seems you're returning an array from the server and trying to map this to a SwiftDictionary ([String : Any?]). If you need to use a JSON type (as opposed to changing the schema to something more specific), you'll have to use public typealias JSON = Any.

@niraj1991
Copy link

@martijnwalraven if I am writing public typealias Json = Any Then I get the below error.
public typealias Json = Any

Screenshot 2019-03-28 at 6 31 22 PM

Here is schema file.

       {
            "name": "reactiondata",
            "description": null,
            "args": [],
            "type": {
              "kind": "SCALAR",
              "name": "Json",
              "ofType": null
            },
            "isDeprecated": false,
            "deprecationReason": null
          },

Please let me know if I need to change anything.

@martijnwalraven
Copy link
Contributor

@niraj1991 Ah, my mistake, I don't think Any is supported as a custom scalar type. Honestly, this doesn't seem like a great use case for custom scalars, you should really give reactiondata a proper type.

@designatednerd designatednerd added the codegen Issues related to or arising from code generation label Jul 10, 2019
@JakeTorres
Copy link

Sript phase: SCRIPT_PATH="${PODS_ROOT}/Apollo/scripts"
cd "${SRCROOT}/${TARGET_NAME}"
"${SCRIPT_PATH}"/check-and-run-apollo-cli.sh codegen:generate --target=swift --includes=./**/*.graphql --passthroughCustomScalars --localSchemaFile="schema.json" API.swift

I do it in: JSONStandardTypeConversions
typealias Long = Int64
extension Long: JSONDecodable, JSONEncodable {
public init(jsonValue value: JSONValue) throws {
guard let number = value as? NSNumber else {
throw JSONDecodingError.couldNotConvert(value: value, to: Int64.self)
}
self = number.int64Value
}

public var jsonValue: JSONValue {
    return self
}

}

but run API.Swift get an error: Use of unresolved identifier 'Long'

who can tell me what wrong with that?

@designatednerd designatednerd added this to the 0.14.0 milestone Aug 5, 2019
@designatednerd
Copy link
Contributor

Int support for custom scalars has (finally) shipped in 0.14.0.

@JakeTorres Can you please take your question to our Spectrum chat? it's not quite related to this issue. Thank you!

BobaFetters pushed a commit that referenced this issue Sep 12, 2023
git-subtree-dir: apollo-ios
git-subtree-mainline: 335653a
git-subtree-split: 7426485
BobaFetters pushed a commit that referenced this issue Sep 12, 2023
git-subtree-dir: apollo-ios-codegen
git-subtree-mainline: 97618d5
git-subtree-split: 48b6461c7a4868c960fd82d4935327361b4350f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codegen Issues related to or arising from code generation
Projects
None yet
Development

No branches or pull requests