consider memoizing getArgumentValues

[mnpenner]

Maybe I don't understand something, but I didn't think it would be necessary for GraphQLScalarType.parseLiteral to be called every time a field argument is resolved.

e.g., given this GraphQL query:

{
  clients(limit: 10) {
    id,
    fullName,
    birthDate,
    deathDate,
    age(asOf: "Jan 1st, 1987 @ 04:12:34.456 -0500")
  }
}

And this field:

export default new GraphQLObjectType({
    name: 'Client',
    description: `Person served`,
    fields: () => ({
        age: {
            type: GraphQLString,
            description: `Age in "#y #m" format`,
            args: {
                asOf: {
                    type: DateTime,
                    description: `Date to calculate age from`
                }
            },

            resolve: (client, {asOf}) => {
                dump(asOf.format('D-MMM-YYYY @ H:mm:ss.SSS Z'));
                ...

And this custom type:

export default new GraphQLScalarType({
    name: "DateTime",
    description: `Scalar type representing a date and time with offset, serialized as a string in ISO-8601 date format.\n\ne.g. \`"2016-05-05T20:16:06Z"\``,
    serialize(value) {
        let date;
        if(_.isNumber(value)) {
            if(value === 0) {
                return null;
            }
            date = moment.unix(value);
        } else {
            date = moment(value);
        }
        if(!date.isValid()) {
            throw new GraphQLError(`Serialization error: ${value} is not a valid date`)
        }
        return date.format();
    },
    parseValue(value) {
        // see https://gist.github.com/olange/f6c57d3ca577955fc3a51aa62f88c948
        // or https://github.com/soundtrackyourbrand/graphql-custom-datetype/blob/master/datetype.js
        // but parse it with moment.js
        throw new GraphQLError(`parseValue(${value}) not implemented`)
    },
    parseLiteral(ast) {
        if(ast.kind !== Kind.STRING && ast.kind !== Kind.INT) {
            throw new GraphQLError(`Parse error: expected date string, got ${JSON.stringify(ast.value)}`, [ast]);
        }

        let result = parseDate(ast.value);

        if(!result.isValid()) {
            throw new GraphQLError(`Invalid date: ${JSON.stringify(ast.value)}`);
        }

        return result;
    }
});

The asOf argument is parsed every time resolve is called -- i.e., 10 times for this one query because I've limited the results to 10. It so happens that my parseDate function is slow, this is kind of painful. I can memoize it on my end, but I didn't think that should be necessary.

Why aren't all the literals parsed just once when the query is received?

[leebyron]

Sounds like there's an opportunity for memoization here.

[mnpenner]

Isn't the GraphQL query converted into an AST? And aren't all literals parsed at that time? This would be necessary for error-checking. I'm just wondering why the parsed literal isn't already in memory by the time resolve() is called.

[yaacovCR]

graphql-js/src/execution/execute.ts

Lines 520 to 525 in 2562ce0

	// TODO: find a way to memoize, in case this field is within a List type.
	const args = getArgumentValues(
	fieldDef,
	fieldNodes[0],
	exeContext.variableValues,
	);

Just looking at it quickly, you could use memoize3, but it should be possible to use something like memoize2 as the fieldDef is always the same for that field node.