astFromValue fails with a custom scalar serializing to an object value about graphql-js HOT 5 OPEN

@yaacovCR I agree it will be solved by #3814 , but the seems like a bigger refactor.

I think there's room for improvement in the current impl, without refactoring it. I tend to agree with @ardatan 's take that this seems like a bug that should be fixed.

from graphql-js.

I think this would be solved alternatively by: #3814 .

The relevant code bit would be:

    const literal =
      arg.defaultValue.literal ??
      valueToLiteral(arg.defaultValue.value, arg.type);

where the first condition reads the literal from the preserved literal if the schema was created from an AST, and the second bit converts the value to a literal in a type-safe manner if the schema was created programattically.

see
https://github.com/yaacovCR/graphql-executor/blob/57e32d78041e0263991b7ae2488c62fde32ae930/src/utilities/printSchema.ts#L261-L263

from graphql-js.

So your PR introduces a new method in the leaf types called valueToLiteral right?
Looks more elegant actually. But your PR looks a big change. Maybe we can try to fix this area specifically first before a huge refactor. What do you think?

from graphql-js.

There is also this issue which is a bit related;
#4088
@yaacovCR I am curious what do you think?

from graphql-js.

Yes I think it should also be covered => custom scalars may have to define custom valueToLiteral methods

just to be clear, while my name is on the rebased PR, all the work is from @leebyron (and the feedback he received from others, of course) — my role has just been trying to keep it alive by rebasing it periodically.

I hope it eventually gets in!

from graphql-js.