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

@defer: Validation rule for the unicity of path+label #3938

Closed
BoD opened this issue Mar 17, 2022 · 4 comments
Closed

@defer: Validation rule for the unicity of path+label #3938

BoD opened this issue Mar 17, 2022 · 4 comments
Assignees

Comments

@BoD
Copy link
Contributor

BoD commented Mar 17, 2022

For each @defer directive, ensure:

  • if a label is present, it must be unique across the whole document
  • if not, the path of where it is used must be unique

Example:

query Query1 {
    computers {
        ...ComputerFields @defer #path is computer
        ...ComputerFields2 @defer #path is computer
    }
}

2 directives have the same path computer and no label. A label must be set on at least one of them.

Less obvious example:

query Query1 {
    computers {
        ...ComputerFields @defer(label: "a")
        ...ComputerFields2 @defer(label: "b")
    }
}

fragment ComputerFields on Computer {
  screen {
    ...ScreenFields @defer # path is computer.screen
  }
}

fragment ComputerFields2 on Computer {
  screen {
    ...ScreenFields @defer # path is computer.screen
  }
}

2 directives have the same path computer.screen and no label. A label must be set on at least one of them. To detect this, the context of all the places where the fragment is used is needed...

@BoD BoD self-assigned this Mar 17, 2022
@BoD BoD mentioned this issue Mar 17, 2022
@BoD
Copy link
Contributor Author

BoD commented Mar 17, 2022

On 2nd thought, not sure the 2nd example makes sense actually. Due to how each fragment has its independent Adapter this "deep" check may not be needed. To be confirmed!

@martinbonnin
Copy link
Contributor

Initial thoughts are that it is needed or else each "ComputerFields" adapter cannot know whether the ScreenFields are for themselves or the other one.

@BoD
Copy link
Contributor Author

BoD commented Mar 17, 2022

I think you're right!

2nd example tweaked a bit:

query Query1 {
    computers {
        ...ComputerFields1 @defer(label: "a")
        ...ComputerFields2 @defer(label: "b")
    }
}

fragment ComputerFields1 on Computer {
    screen {
        ...ScreenFields1 @defer
    }
}

fragment ComputerFields2 on Computer {
    screen {
        ...ScreenFields2 @defer
    }
}

fragment ScreenFields1 on Screen {
    isColor
}

fragment ScreenFields2 on Screen {
    resolution
}

Received payloads:

// 0
{"data":{"computers":[{},{}]},"hasNext":true}
// 1
{"data":{"screen":{}},"path":["computers",0],"label":"a","hasNext":true}
// 2
{"data":{"screen":{}},"path":["computers",0],"label":"b","hasNext":true}
// 3
{"data":{"resolution":"640x480"},"path":["computers",0,"screen"],"hasNext":true}
// 4
{"data":{"screen":{}},"path":["computers",1],"label":"a","hasNext":true}
// 5
{"data":{"screen":{}},"path":["computers",1],"label":"b","hasNext":true}
// 6
{"data":{"resolution":"640x480"},"path":["computers",1,"screen"],"hasNext":true}
// 7
{"data":{"isColor":false},"path":["computers",0,"screen"],"hasNext":true}
// 8
{"data":{"isColor":false},"path":["computers",1,"screen"],"hasNext":false}

After receiving 3, inside the adapter for ComputerFields1, the test to read the fields for ScreenFields1 is "have we received the fragment for path computer.0.screen and label null?", the answer is yes, it will try to read isColor which is not there. ❌

@martinbonnin
Copy link
Contributor

Life is hard 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants