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

feat(GraphQL): Provide ability to specify operationName #1224

Merged
merged 2 commits into from
Nov 27, 2023

Conversation

Harsgalt86
Copy link
Contributor

-- Background --
By default when using Graphql (Apollo), all network requests hit the same endpoint, and as such they all have the same name in the devtools. e.g: graphql. This makes performance monitoring and diagnosing problematic queries difficult. To get around this, we append the operationName of a graphql query to the url. e.g graphql?GetUsers, graphql?getUserData so we can uniquely identify each query.
https://stackoverflow.com/questions/64642948/is-there-a-way-to-name-graphql-requests-in-the-devtool-network-tab

-- Problem --
The dynamically built query in slickgrid does not allow specifying an operationName and so all queries generated by slickgrid have an operationName of undefined
eg: query { datasetName (....) }
https://stackoverflow.com/questions/57049366/get-query-mutation-operation-name

As such, all the slickgrid queries look like graphql?undefined when looking in devtools.

-- Solution --
Provide an optional GraphQlOption operationName that if provided will be used during GraphQlService.buildQuery
eg: query operationName { datasetName (...) }

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d3e871d) 100.00% compared to head (68d7284) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1224   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          245       245           
  Lines        17277     17277           
  Branches      6247      6248    +1     
=========================================
  Hits         17277     17277           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding ghiscoding changed the title feat: Provide ability to specify operationName feat(GraphQL): Provide ability to specify operationName Nov 27, 2023
@ghiscoding ghiscoding merged commit 4db6c34 into ghiscoding:master Nov 27, 2023
5 checks passed
@ghiscoding
Copy link
Owner

ghiscoding commented Nov 27, 2023

that looks fine but I would like to know if you have many more changes like this? Because I don't want to publish new version every week (I published the other PR over the weekend already), especially since it's only GraphQL related 😝

@Harsgalt86
Copy link
Contributor Author

I would like to know if you have many more changes like this

I don't have anything planned, but we are in testing phase + new devolopment phase so something might pop up.

I don't want to publish new version every week

Yeah, no worries, this change isn't urgent. If anything does pop up I'll avoid staggering the PRs and open them together (individually) so they can be released at once.

Please don't feel the need to release on my account :)

I published the other PR over the weekend already

Thank you very much for being so responsive.

@ghiscoding
Copy link
Owner

ghiscoding commented Nov 27, 2023

Yeah no worries, my main focus at the moment is the next major version which I hope to release before Christmas. See Slickgrid-Universal Roadmap 4.0. While working on this, I found that there are things that I could do in current version but for the most part it's only the next branch.

So I will probably wait until next weekend to release your new code. Another thing about code change in Slickgrid-Universal is that at the moment, anytime I publish a new version of Slickgrid-Universal, I have to release all downstream libs (Angular-Slickgrid, Aurelia-Slickgrid, ...) because of a TypeScript issue that I have with the lib, I think it's related to the fact that I extends GridOption for custom grid options in Aurelia-Slickgrid and others and that causes TypeScript to not be happy when I ship new version of Slickgrid-Universal without downstream libs since that causes build errors with Type mismatch, so at the moment I have to release all libs on the same day, I have a possible idea on how to fix that in the next major release but in the meantime I have to publish many projects on the same day, hence why I was asking if you had more things to bring 😉

At least now, since last weekend, I ship new releases via GitHub Action in CI workflow to provide npm provenance. So it is becoming quicker to release, so there's that 😄

@Harsgalt86
Copy link
Contributor Author

I don't have anything planned

Spoke to soon, I'm looking into disabling the "select all" checkbox when there are no rows or all the rows are disabled. Hoping to get that in before your release this weekend.

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

Successfully merging this pull request may close these issues.

2 participants