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

converting null to undefined #2412

Closed
obartra opened this issue Oct 28, 2017 · 18 comments
Closed

converting null to undefined #2412

obartra opened this issue Oct 28, 2017 · 18 comments
Labels
🙏 help-wanted 🚧 in-triage Issue currently being triaged

Comments

@obartra
Copy link

obartra commented Oct 28, 2017

Intended outcome:

In Apollo 2, I'm trying to convert all null values to undefined in every query response.

Actual outcome:

I get a console warning saying the field is missing. It looks like:

writeToStore.js:114 Missing field locationType in {
  "__typename": "Offense",
  "id": "77205221-5f74-45b9-5268-67f96b55beb4",
  "kind": "INTIMIDATION"

I also get no response back and no errors

How to reproduce the issue:

This is how I define my link:

const httpLink = createHttpLink({
  uri: window.xnGlobals.ENDPOINT_GQL,
  fetch,
})
const nullToUndefined = value => {
  if (isPlainObject(value)) {
    return mapValues(value, nullToUndefined)
  }
  if (isArray(value)) {
    return value.map(nullToUndefined)
  }
  if (value === null) {
    return undefined // THIS SHOULD BE UNDEFINED
  }
  return value
}
const nullLink = new ApolloLink((operation, forward) => forward(operation).map(nullToUndefined))
const link = nullLink.concat(httpLink)

Version

  • apollo-client@2.0.0
@jbaxleyiii
Copy link
Contributor

hmm @obartra I do believe that we treat undefined as bad responses (missing fields) since the GraphQL spec only allows for returning nulls in the case of empty data.

What is your use case?

@obartra
Copy link
Author

obartra commented Oct 28, 2017

I'm trying to simplify how I handle default values. undefined is used for ES6 default parameters, React default props and applying defaults in other libraries like lodash. So it's a more convenient way to indicate a value has not been set for me. Otherwise I need to manually check and apply default values.

I assumed link allowed me to modify a response after it was processed by apollo-client (so that apollo client would use null/undefined internally the same way). Maybe I'm using the wrong method? Is there another hook to modify the response without affecting the library?

If that's not available, I'm not familiar with the implementation details, but would it be possible to detect if a field is present with hasOwnProperty or 'key' in object instead?

@jbaxleyiii
Copy link
Contributor

@obartra I think we can probably make those adjustments! The first step would be to write a failing test showing how undefined is not handled currently. Would you be up for doing that?

@obartra
Copy link
Author

obartra commented Oct 31, 2017

For sure, how do you want to approach it? Do you want me to add tests for supporting this ApolloLink or to use hasOwnProperty / 'key' in object instead of undefined?

I don't see many tests for link other than on __tests__/QueryManager/links.ts do you want them there or is there a better spot?

@obartra
Copy link
Author

obartra commented Nov 7, 2017

Hey @jbaxleyiii, not to nag you but if you can give me some direction on the issues ^ I can get started with some test cases

@stale
Copy link

stale bot commented Nov 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@obartra
Copy link
Author

obartra commented Nov 29, 2017

My current approach is to do the following:

const nullToUndefined = value => {
  if (_.isPlainObject(value)) {
    return _.mapValues(value, nullToUndefined)
  }
  if (_.isArray(value)) {
    return value.map(nullToUndefined)
  }
  if (value === null) {
    return undefined
  }
  return value
}
const internal = new ApolloClient({ /* ... */ })

export const client = {
  ...internal,
  mutate: (...args) => internal.mutate(...args).then(nullToUndefined),
  query: (...args) => internal.query(...args).then(nullToUndefined),
}

It seems to do the trick but it's far from a clean solution

@menkveldj
Copy link

I have a similar problem. I was able to solve it with a utility function that passes only into the request variables:

utils.js

// handle empty string
export const graphqlFriendlyObject = x => Object.keys(x).reduce((p, c) => {
  p[c] = x[c] || undefined;
  return p;
}, {});

And then in my mutation:

import {graphqlFriendlyObject} from "../../Utils";
...
graphql(MutationUpdateChild, {
    options: {
      refetchQueries: [{ query: QueryAllChildren }],
      update: (proxy, { data: { updateChild } }) => {
        const query = QueryAllChildren;
        const data = proxy.readQuery({ query });

        // find item to update
        const updateIndex = data.listChildren.items.findIndex(
          child => child.id === updateChild.id
        );

        // splice in update
        data.listChildren.items.splice(updateIndex, 1, updateChild);
        proxy.writeQuery({ query, data });
      }
    },
    props: props => ({
      updateChild: child => {
        const fChild = graphqlFriendlyObject(child);

        return props.mutate({
          variables: { ...fChild, expectedVersion: fChild.version },
          optimisticResponse: () => ({
            updateChild: {
              ...child,
              __typename: "Child"
            }
          })
        });
      }
    })
  }),

@hwillson hwillson added to do and removed to do labels Sep 5, 2018
@kamaltmo
Copy link
Contributor

kamaltmo commented Sep 6, 2018

Any news on this front? Looking at wanting this behavior too but can't find a good way to implement it across the whole client for consistent behaviour. @obartra solution doesnt work with react-apollo as they appear to use watchQuery.

@obartra
Copy link
Author

obartra commented Sep 6, 2018

If the issue is with the <Query> component we overwrite it as well with:

export function Query<R: {}>({ children, ...props }: Props<R>) {
  return (
    <ReactApolloQuery fetchPolicy="cache-and-network" {...props}>
      {(response: any = {}) => children(((nullToUndefined(response): any): R))}
    </ReactApolloQuery>
  )
}

But all these are just hacky workarounds. It would be nice to be able to modify the responses of all queries and mutations directly from Apollo middleware

@kamaltmo
Copy link
Contributor

kamaltmo commented Sep 7, 2018

Yeah that would be ideal, even the ability to allow partial matches to return responses would allow the link solution to work.

@TimoRuetten
Copy link

I get your fix with the function @obartra and it helped a lot. But how you solve this with the graphql hoc ? Are you doing a simple props() override or something before this ?

@philiptzou
Copy link

hmm @obartra I do believe that we treat undefined as bad responses (missing fields) since the GraphQL spec only allows for returning nulls in the case of empty data.

What is your use case?

@jbaxleyiii Hi can you point out where is the code ("treat undefined as bad responses") is? I encountered a similar problem because of the server-side program (GraphQL-Java drops all null fields).

@jbaxleyiii jbaxleyiii added the 🚧 in-triage Issue currently being triaged label Jul 9, 2019
@jbaxleyiii
Copy link
Contributor

Thanks for reporting this. There hasn't been any activity here in quite some time, so we'll close this issue for now. If this is still a problem (using a modern version of Apollo Client), please let us know. Thanks!

@jtomaszewski
Copy link

Is there a thread somewhere on Apollo in which you discuss whether you'd be willing to support adding an option to the apollo server/client, that would replace all nulls with undefineds? For TS devs, the difference between null and undefined is meaningless, and that's why most of TS devs have dropped the null type completely.

Without that, currently we're forced to either use nulls (just because GQL client/server uses them), or use helpers like this one:

type RecursivelyReplaceNullWithUndefined<T> = T extends null
  ? undefined // Note: Add interfaces here of all GraphQL scalars that will be transformed into an object
  : T extends Date
  ? T
  : {
      [K in keyof T]: T[K] extends (infer U)[]
        ? RecursivelyReplaceNullWithUndefined<U>[]
        : RecursivelyReplaceNullWithUndefined<T[K]>;
    };

/**
 * Recursively replaces all nulls with undefineds.
 * Skips object classes (that have a `.__proto__.constructor`).
 *
 * Unfortunately, until https://github.com/apollographql/apollo-client/issues/2412
 * gets solved at some point,
 * this is the only workaround to prevent `null`s going into the codebase,
 * if it's connected to a Apollo server/client.
 */
export function replaceNullsWithUndefineds<T>(
  obj: T
): RecursivelyReplaceNullWithUndefined<T> {
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  const newObj: any = {};
  Object.keys(obj).forEach((k) => {
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const v: any = (obj as any)[k];
    newObj[k as keyof T] =
      v === null
        ? undefined
        : // eslint-disable-next-line no-proto
        v && typeof v === "object" && v.__proto__.constructor === Object
        ? replaceNullsWithUndefineds(v)
        : v;
  });
  return newObj;
}

@organics2016
Copy link

期待这个问题能被正确的解决

@dwesty17
Copy link
Contributor

Is there a generally accepted answer to this? It's quite annoying to have to deal with this on the client side when using Typescript. Reading through the thread it seems like this should still be open.

@ackvf
Copy link

ackvf commented Nov 29, 2022

It's actually very annoying. In typescript an optional field is defined as something?: Type which translates to something: Type | undefined. null messes this up big time by forcing us to use something?: Type | null everywhere. What a mess.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🙏 help-wanted 🚧 in-triage Issue currently being triaged
Projects
None yet
Development

No branches or pull requests