Fixup string bool handling #212
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Start out simple. Assume we cannot return another defer from the defer we already got. This makes for a relatively simple update to the code base, where the result of a field closure applies the chain of functions before it works with the result at a deeper level.
Strategy: keep a chain of changes. Apply this chain when we have value resolution. Do not cope with the chain dumping a defer for now, but it is doable given the closure handler anyway. It has to work with the possibility that the upstream ref changes anyway, so it should be sound to make this change on top.
Strategy: When a chain returns a defer in the middle, build a new field closure using the standard closure building tooling. Then ask the upstream to change its reference to this new closure. Also graft the completion chain on top of the chain we got from the newly formed closure.
Special handling of variable coercion is necessary in GraphQL because if the schema specifies a type [A] then it is valid to use the type A in its place (for convenience reasons). However, the code did not contain the correct congruence relations. Coercion of [A] with [B] is the same as running a coercion of A with B, hoping that works out. Add the missing congruence rule, then also alter test cases so they cover this particular problem. While here, reorder names to be consistent with the type checker.
Step one of a cleanup. Use a default of `undefined` over the default of `null` in the input. This opens up making a discrimination between default values that were never given by the user (`undefined`) and values which were set to the value `null` by the user.
Rather than passing a not_found on from a failed maps:get/3, handle it locally in the parameter checker. This avoids a pattern match later on in the code, which then streamlines the whole thing into a more fluid flow. This refactoring is needed for the overarching idea of handling parameter passing correctly in the system.
The fact everything is a #vardef{} record means we can match on it
early and then handle things naturally in the flow.
We had two checking paths for not-found objects. Coalesce these into a single path.
The check_param judgement was equivalent to the check_value judgement except for enum values. Introduce a `sub_context` of either being in query or variable mode when checking values. Then use this to discriminate between the enum values, while streamlining every other flow to the be the same. This kills a lot of type checker lines and removes a lot of redundancy in the type checker.
When we have a default parameter, handle that case gracefully and let the caller decide on how to proceed with default parameters. This allows default parameters to propagate from a variable into a field, if the parameter isn't given by the caller. The parameter thus overrides the underlying game.
The failing test case allows us to properly fix the problem we've seen internally at Shopgun.
Do not let the Parser override the default value of the record. This makes sure we only have to fix stuff in one place in the future and also plugs an error in the system.
Now, a #var{} record is annotated with its underlying default coerced
value if applicable. So when a variable is omitted, and has no default
value, we pick up the coerced value from the underlying arg in the
execution phase. This fixes the bug.
It doesn't hit the case I'm interested in yet, but is a start
Interally, we can have floating point gray scale values. These should be output as integer-rounded values.
We need a way to supply a colorType. This query is it.
Remove take_arg/3 and just fold the lists:keytake/3 into the caller directly. While here, identify that we are doing a call to sub with the same type, which will always succeed, so remove this redundant check. This paves the way for correctly handling values.
This fixes a problem in the system where we correctly use defaults for parameters inside input objects, but don't when they are used in arguments. This will use defaults correctly in arguments as well, by augmenting the variable with the underlying default value.
When you default arguments, inject `undefined`. This then allows us to recast that into a null value later on in the system.
Fix/default args coercion
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bool type definition:
JSON true and false are fine. but strings "true" and "false" occur error now.
I fix it up