Implement FieldMerge and FieldMergeFunction

[dfalling]

I feel like this is pretty close. Something's wrong with the @unboxed though:

  open ApolloClient
  make(
    ~cache=Cache.InMemoryCache.make(
      ~typePolicies=[
        (
          "Trip",
          Cache.InMemoryCache.TypePolicy.make(
            ~fields=[
              (
                "elements",
                FieldPolicy({
                  merge: Some(MergeFunction(incomingMerge)),
                  read: None,
                  keyArgs: None,
                }),
              ),
            ],
            (),
          ),
        ),

Compiles to

ApolloClient.make(undefined, undefined, undefined, Caml_option.some(httpLink), ApolloClient__Cache_InMemory_InMemoryCache.make(undefined, undefined, undefined, undefined, [
          [
            "Trip",
            ApolloClient__Cache_InMemory_Policies.TypePolicy.make([[
                    "elements",
                    {
                      TAG: /* FieldPolicy */3,
                      _0: {
                        keyArgs: undefined,
                        read: undefined,
                        merge: /* MergeFunction */{
                          _0: incomingMerge
                        }
                      }
                    }
                  ]], undefined, undefined, undefined, undefined, undefined)
          ]

How can I get rid of that extra merge: { _0: incomingMerge }?

[dfalling]

I had to move this type further down in the module to share the FieldFunctionOptions.t type.

[jeddeloh]

I'd have to look close at the typescript types, but on first glance this looks great.

How can I get rid of that extra merge: { _0: incomingMerge }?

As long as you're keeping with the style of the rest of the library you can't. This is the cost of (what I considered at the time) a better API. So we can do stuff like merge: MergeFunction(someFn) vs. merge: FieldMerge.mergeFunction(someFn). My opinion was that the configuration of this stuff was never going to be a performance issue and it was so much more ergonomic and clear to use a variant instead of hunting down constructor functions. In other words, if we dropped the variants entirely and went with the unboxed stuff directly we'd get what you expect.

[dfalling]

Unfortunately that nesting breaks the feature. Apollo doesn't recognize the merge function and nothing happens. I need a way for the function to be assigned directly, without the _0.

My understanding of @unboxed is that it's supposed to do that.

[jeddeloh]

I think the unboxed portion is working fine. The _0 is unwrapped when passed to the FieldMerge.toJs here.

I think that the actual issue is that line 124 needs to be typed as a Js_.t

- let mergeFunction = (v: FieldMergeFunction.t<'existing>) => Any(v)
+ let mergeFunction = (v: FieldMergeFunction.Js_.t<'existing>) => Any(v)

That will then force you to call FieldMergFunction.toJs on 133

[dfalling]

I think that the actual issue is that line 124 needs to be typed as a Js_.t

Aah, ok. I fixed those references, but still am seeing the _0. I've cleaned the package and my entire codebase so I don't think it's a caching issue.

One additional issue- the true isn't being output as true. It's output as merge: /* True */0 which actually results in false, since 0 is falsey.

[jeddeloh]

In the compiled file where you configured your client (what you showed previously), you should still be seeing _0. Also, merge: /* True */0 is the correct representation of FieldMerge.True at that level. All of that is passed to InMemoryCache.make and then goes through a bunch of layers of conversion from the ReScript layer. If you jump through the compiled code it's a little obtuse, but it should eventually see that 0 get converted to true just before it gets passed to the actual Apollo function in javascript.

What have you tried in debugging the issue so far? Does a debugger in your merge function get called? You could also put some debuggers inFieldMerge.toJs for instance, to see what is actually going on there if you don't believe it's getting converted.

[dfalling]

Oh got it- I missed the fact that the toJs step happens afterwards. Turns out my issue was with yarn link not picking up my changes. I ran it off my fork instead and it worked as expected. Thanks for the help working through this! Verified that MergeFunction and True work locally as expected.

[jeddeloh]

Excellent! I really appreciate you putting in the PR. I'll merge and cut a release soon.