Shane Krueger

Results 854 comments of Shane Krueger

Interesting. Are you sure data loaders execute in reverse order with a serial execution strategy? They shouldn't; it's a Queue not a Stack. See: https://github.com/graphql-dotnet/graphql-dotnet/blob/6b9af75f9e891677dcf7f4c35ee7ae2f2cc2a935/src/GraphQL/Execution/SerialExecutionStrategy.cs#L28 and https://github.com/graphql-dotnet/graphql-dotnet/blob/6b9af75f9e891677dcf7f4c35ee7ae2f2cc2a935/src/GraphQL/Execution/SerialExecutionStrategy.cs#L50 and https://github.com/graphql-dotnet/graphql-dotnet/blob/6b9af75f9e891677dcf7f4c35ee7ae2f2cc2a935/src/GraphQL/Execution/SerialExecutionStrategy.cs#L61

Note that chained dataloaders should execute after all pending data loaders execute, so that the 'first level' data loaders can all queue up the 'second level' data loaders. https://github.com/graphql-dotnet/graphql-dotnet/blob/6b9af75f9e891677dcf7f4c35ee7ae2f2cc2a935/src/GraphQL/Execution/SerialExecutionStrategy.cs#L74

@mbenedykconfigit Would you be interested in adding documentation? A PR would be welcome.

Oh I see you're not logging when the data loader executes, but rather when the child nodes execute. Hmm, it shouldn't be in reverse order either though. Could be a...

Oh because the array nodes are in a data loader, the data loader nodes execute in the correct order, pushing their children into the stack in the correct order, but...

I believe the correct result SHOULD be: ``` root root.families root.families[0] root.families[0].leader_dataLoader (DataLoader) root.families[0].leader root.families[0].leader.lastName root.families[0].leader.name root.families[1] root.families[1].leader_dataLoader (DataLoader) root.families[1].leader root.families[1].leader.lastName root.families[1].leader.name root.families[2] root.families[2].leader_dataLoader (DataLoader) root.families[2].leader root.families[2].leader.lastName root.families[2].leader.name root.families[3] root.families[3].leader_dataLoader...

> I'm starting my vacation tomorrow, so I won't have time to create PR with suggestions/documentation, let's get back to it after them ;) No problem; thanks!

I like the concept @sungam3r - however, this does not comply with SemVer 1.0.0 or SemVer 2.0.0. Namely, it does not allow strings but only numbers. The `Version` class was...

> So maybe just change comments and remove mention about SemVer Ok

See also: https://www.nuget.org/packages/NuGet.Versioning