Add fragment variables to parseLiteral

[JoviDeCroock]

This adds in support for parsing literals that are encapsulated within a fragment, this allows us to leverage any shadowed operation variable or newly introduced fragment-variable.

[netlify]

✅ Deploy Preview for compassionate-pike-271cb3 ready!

Name	Link
🔨 Latest commit	28f0bf2
🔍 Latest deploy log	https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/66e28f5e48049600086de0f5
😎 Deploy Preview	https://deploy-preview-4189--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[github-actions]

Hi @JoviDeCroock, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

• @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
• @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

[yaacovCR]

@JoviDeCroock -- we may possibly need to decide a gameplan on #4182 before we address this?

If we keep what we have now without #4182, I think we would need to pass both variableValues and fragmentVariables to .parseLiteral()

In the default value validation stack, in #3812 @leebyron has added a replaceVariables() helper that I suggest we extract a bit early, and make it so that parseLiteral() takes a single const value, without variables or fragmentVariables., and then we don't have to modify the signature twice when that work hopefully lands, and we don't have to bother Scalar implementers with having to understand anything about fragmentVariables.

On the other hand, if we take #4182, I think the variableValues would already be coalesced, so we might not need this PR? Have to double check, but either way would be either this PR or no PR, different different than the above plan if we stick with what we have now.

My preference would be to close #4182 and keep what we have -- see discussion there in terms of benchmarking -- and go with extracting replaceVariableValues(). I can try to work on that as a comparison.

As an aside, replaceVariableValues() is not a necessary part of #3812 or -- as far as I can tell -- the entire stack. Following the discussion on #3812, it seems like it was a necessary part of an earlier iteration of that stack.

[yaacovCR]

Woops, replaceVariableValues() does indeed depend on valueToLiteral(). My mistake. So it can't be simply extracted, we will have to go through the stack carefully. But I think it would be worth it to get default value validation in!

[JoviDeCroock]

@yaacovCR I am with you on closing the other PR, I don't understand why we need replaceVariables, the merging of fragment with normal variables feels very sane. What is the underlying issue/ reason for this?

[yaacovCR]

Merging the variable maps naively means that if fragment and operation variable names overlap, and the fragment variable is not provided, the operation variable will be used instead.

I have now successfully rebased the first four PRs in the default value validation stack #3809 #3810 #3811 and #3812

#3812 brings sanity to variables embedded within scalars by way of a new replaceVariables helper — and fixes this issue.