Giter Site home page Giter Site logo

Comments (12)

calvincestari avatar calvincestari commented on July 4, 2024

Hi @solidcell - can you share the code you've got so far for your custom scalar?

I agree moving to 1.0 is a large migration; have you read the 1.0 migration guide that lays out everything that you may need to change during the migration?

from apollo-ios.

solidcell avatar solidcell commented on July 4, 2024

Yea I've already read the migration guide.

The easiest way I've found to get a first pass which compiles, and sort of works is to not use a typealias directly on [String:Any], but to make it a wrapper struct. However that has its disadvantages. Here's what I've got so far:

public struct JSON: CustomScalarType, SelectionSetEntityValue {

    public init(_fieldData: AnyHashable?) {
        guard let _fieldData else {
            fatalError()
        }
        self.value = _fieldData
    }

    public init(_jsonValue value: JSONValue) throws {
        self.value = value
    }
    
    public init(anyHashable: JSONValue) {
        self.value = anyHashable
    }

    public var _fieldData: AnyHashable {
        value
    }

    public var _jsonValue: JSONValue {
        value
    }

    public var _asAnyHashable: AnyHashable {
        value
    }

    public subscript<T>(_ key: String) -> T? {
        guard let dict = value as? [String : Any],
              let casted = dict[key] as? T
        else { fatalError() }
        return casted
    }

    public func compactMap<T>(
        _ transform: ((key: String, value: Any)) throws -> T?
    ) rethrows -> [T] {
        guard let dict = value as? [String : Any]
        else { fatalError() }
        return try dict.compactMap(transform)
    }

    public func compactMapValues<T>(
        _ transform: (Any) throws -> T?
    ) rethrows -> [String : T] {
        guard let dict = value as? [String : Any]
        else { fatalError() }
        return try dict.compactMapValues(transform)
    }

    public var values: Dictionary<String, Any>.Values {
        guard let dict = value as? [String : Any]
        else { fatalError() }
        return dict.values
    }

    public var description: String {
        guard let dict = value as? [String : Any]
        else { fatalError() }
        return dict.description
    }

    private let value: JSONValue
}

I haven't refactored this up yet since I don't like the direction this solution is going. It's enabled me to get the app compiling and running again, but there are a few issues I've noticed so far:

  1. Needing to redefine and delegate methods (compactMap, subscript, etc.) to the wrapped Dictionary is getting out of hand and isn't elegant.
  2. It's a bit awkward that a JSON field from a result is treated like JSON wrapper type, but if it contains nested JSON itself, that nested JSON accessed with subscript won't technically be JSON (the wrapper type), but just a regular AnyHashable([String : Any]). Not the end of the world, but it's awkward that with this solution the types for top-level and nested JSON hashes are different, so you need to be careful about casting and what to expect. And I don't think wrapping it with the type explicitly upon returning in subscript is great either.

I would much prefer to be able to simply work with the underlying [String : Any] type as I had been doing with Apollo 0.x. I'm hoping I can get back to that. However, trying to do that is giving me a whole host of problems. So I'm not sure if it's the better solution in the end or if it's even possible.

Doing this, for instance:

public typealias JSON = [String : Any]

extension JSON: CustomScalarType {}

Is giving me:

'CustomScalarType' requires the types 'Any' and 'any GraphQLOperationVariableValue' be equivalent
Conditional conformance of type 'Dictionary<Key, Value>' to protocol 'CustomScalarType' does not imply conformance to inherited protocol 'AnyScalarType'
Conditional conformance of type 'Dictionary<Key, Value>' to protocol 'CustomScalarType' does not imply conformance to inherited protocol 'OutputTypeConvertible'
Did you mean to explicitly state the conformance with different bounds?
Type 'Dictionary<Key, Value>' does not conform to protocol 'CustomScalarType'

I'm just starting to dive deeper, but something tells me I'm fighting against the current. The types and protocols in Apollo 1.x are a lot to take in, so I'm just trying to wrap my head around it. For instance, the first error: 'CustomScalarType' requires the types 'Any' and 'any GraphQLOperationVariableValue' be equivalent. The compiler isn't being particularly helpful and it's not clear to me how CustomScalarType is making that requirement in the first place. Maybe some extension being defined somewhere on Dictionary which is influencing this conformance.. It's hard to say.

from apollo-ios.

calvincestari avatar calvincestari commented on July 4, 2024
  1. Needing to redefine and delegate methods (compactMap, subscript, etc.) to the wrapped Dictionary is getting out of hand and isn't elegant.

I'm not sure why you need to be defining conformance to SelectionSetEntityValue, that might be compounding the issues. Any as the element is not helping though, it's too broad. What about using AnyHashable instead?

  struct JSON: CustomScalarType, Hashable {
    private let wrapped: [String: AnyHashable]

    init(_jsonValue value: ApolloAPI.JSONValue) throws {
      guard let value = value as? [String: AnyHashable] else { throw JSONDecodingError.wrongType }

      self.wrapped = value
    }
    
    var _jsonValue: ApolloAPI.JSONValue { wrapped }
  }
  1. It's a bit awkward that a JSON field from a result is treated like JSON wrapper type, but if it contains nested JSON itself, that nested JSON accessed with subscript won't technically be JSON (the wrapper type), but just a regular AnyHashable([String : Any]). Not the end of the world, but it's awkward that with this solution the types for top-level and nested JSON hashes are different, so you need to be careful about casting and what to expect. And I don't think wrapping it with the type explicitly upon returning in subscript is great either.

Can you show me how you ideally want to be able to access the custom scalar value? I'm not sure there is any way around having to cast the type out of the subscript when using the value type of Any, and you will always need to be careful because there is no type safety in Any.

Custom scalars are for your code to define and the type safety you get from that is entirely up to what your custom type provides. If you want that type to be generated for you then it needs to be fully defined in the schema so codegen can work with it.

from apollo-ios.

solidcell avatar solidcell commented on July 4, 2024

If I don't conform to SelectionSetEntityValue, I get this error:
image

Using AnyHashable instead of Any as the value didn't change anything (either better or worse), so I'll leave it as AnyHashable. That was my plan, but I was trying to be as incremental in my migration as possible. So I was leaving it as Any for now until I got everything working again, since that's what it was pre-migration.

I'm not sure there is any way around having to cast the type out of the subscript when using the value type of Any

Ah, misunderstanding here. It's not my intention to get more type safety from Any than I should expect. I was just now trying to type up exactly what I was after, but I was on a wrong path. So nevermind my #2.

In the end, it would just be nice to just have #1. A typealias would allow me to stop having this need to delegate everything and just simply have the type I'm really after. Something like what's possible for Date:

public typealias Date = Foundation.Date

extension Foundation.Date: CustomScalarType {
    public init (_jsonValue value: JSONValue) throws {
        guard let string = value as? String else {
            throw JSONDecodingError.couldNotConvert(value: value, to: String.self)
        }

        guard let date = Date(jsonString: string) else {
            throw JSONDecodingError.couldNotConvert(value: string, to: Date.self)
        }

        self = date
    }

...

But using a typealias gives me:

'CustomScalarType' requires the types 'AnyHashable' and 'any GraphQLOperationVariableValue' be equivalent
Conditional conformance of type 'Dictionary<Key, Value>' to protocol 'CustomScalarType' does not imply conformance to inherited protocol 'AnyScalarType'
Conditional conformance of type 'Dictionary<Key, Value>' to protocol 'CustomScalarType' does not imply conformance to inherited protocol 'OutputTypeConvertible'
Did you mean to explicitly state the conformance with different bounds?
Type 'Dictionary<Key, Value>' does not conform to protocol 'CustomScalarType'

from apollo-ios.

calvincestari avatar calvincestari commented on July 4, 2024

@solidcell - have you managed to make any progress on this or is it still an issue? I'm wondering if the other issue we've worked on been able to help you with this one too?

from apollo-ios.

AnthonyMDev avatar AnthonyMDev commented on July 4, 2024

What if you change your scalar type to [String: AnyHashable]? I think that might work out of the box. We needed to ensure the values were Hashable in 1.0.

from apollo-ios.

calvincestari avatar calvincestari commented on July 4, 2024

@solidcell below is a sample project I put together that uses a custom scalar named JSON of type [String: AnyHashable], including a test getting data out of it to ensure it parses as expected.

CustomScalar-AnyHashable-SampleProject.zip

I'm going to close this issue with supplying the working sample project but if this doesn't suit your needs let me know in another comment on this issue and I'll pick it up again.

from apollo-ios.

github-actions avatar github-actions commented on July 4, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

from apollo-ios.

TizianoCoroneo avatar TizianoCoroneo commented on July 4, 2024
  struct JSON: CustomScalarType, Hashable {
    private let wrapped: [String: AnyHashable]

    init(_jsonValue value: ApolloAPI.JSONValue) throws {
      guard let value = value as? [String: AnyHashable] else { throw JSONDecodingError.wrongType }

      self.wrapped = value
    }
    
    var _jsonValue: ApolloAPI.JSONValue { wrapped }
  }

I independently wrote almost exactly this snippet today to test Contentful's GraphQL API.

public struct JSONScalar: Hashable, CustomScalarType {
    public let values: [String: AnyHashable]

    public init(_jsonValue value: JSONValue) throws {
        guard let object = value as? [String: AnyHashable] else {
            throw JSONDecodingError.couldNotConvert(value: value, to: [String: AnyHashable].self)
        }

        self.values = object
    }

    public var _jsonValue: JSONValue {
        self.values as JSONValue
    }
}

While developing stuff around this I also encountered the same issue that @solidcell mentioned ("JSON must conform to SelectionSetEntityValue"), but I noticed that adding a conformance to Hashable fixes all issues. After a clean. And a "Reset package caches". And closing/reopening Xcode for good measure

from apollo-ios.

calvincestari avatar calvincestari commented on July 4, 2024

While developing stuff around this I also encountered the same issue that @solidcell mentioned ("JSON must conform to SelectionSetEntityValue"), but I noticed that adding a conformance to Hashable fixes all issues.

We have this section in the Custom Scalars documentation that talks about Hashable conformance but that makes it sound optional rather than required. We may need to update that be more explicit that but I'll take a look at the code too and ensure we're not missing anything.

from apollo-ios.

solidcell avatar solidcell commented on July 4, 2024

@calvincestari Thanks for the example.

a custom scalar named JSON of type [String: AnyHashable]

Maybe I'm misunderstanding this, but that's not quite true. It's still a wrapper around [String: AnyHashable], not [String: AnyHashable] itself. This is unlike what I've seen done for something like Date:

public typealias Date = Foundation.Date

extension Foundation.Date: CustomScalarType {
    // ....

Your code improves the implementation I had going, but unfortunately since it's still really just wrapping a [String: AnyHashable] and not [String: AnyHashable] itself, I can't subscript on a JSON directly. So for instance, this won't work:

model.everything?.first?.custom?["aKey"] (nor compactMap, keys, etc.)

So I'm still having to delegate to the underlying wrapped with a bunch of repetitive shim methods.

Also, since I'm getting back a JSON?, it's way more likely that I'll make a mistake once I start diving into it. To clarify, custom returns me a JSON?, but then once I subscript (with my shim method on JSON) to get the value at "lastKey", I now get an AnyHashable? which is really a [String:String], not a JSON? wrapping a [String:String]. So assuming I get the subscript shim in place, it's unfortunate that doing something like the following will start to be an issue only tests or manual QA will uncover:

if let nestedJSON = model.everything?.first?.custom?["lastKey"] as? JSON {
    // BUG! After updating Apollo and the implementation of JSON, we'll never actually go in here anymore, since the value for "lastKey" is not actuallly `[String: String]` anymore, but a wrapper around it.
    // So now I need to always be careful if I mean `JSON` or `[String: AnyHashable]` and I need to carefully comb my current codebase for changes to make like this.
}

An example of one of many changes I've needed to make in our codebase to account for this:

Original (Apollo 0.x):

if let a = json["key"] as? JSON { //...

Now required modification (Apollo 1.x):

if let a = json.wrapped["key"] as? [String : AnyHashable] { //...

Unfortunately I've already run into new bugs that cropped up for this reason. Unfortunately our API uses this JSON scalar at all, but that's just how it is. So hopefully I've found all the places that need to be updated, but I'm also hoping I can just change it back to just using JSON in all the places instead of being stuck in this awkward in-between two types situation.

from apollo-ios.

calvincestari avatar calvincestari commented on July 4, 2024

OK, I understand better what you're getting at now. Let me see what I can get working here..

from apollo-ios.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.