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

Add containsSupergraphSpec #44

Merged
merged 1 commit into from
Feb 22, 2024
Merged

Add containsSupergraphSpec #44

merged 1 commit into from
Feb 22, 2024

Conversation

kamilkisiela
Copy link
Member

@kamilkisiela kamilkisiela commented Feb 22, 2024

This method is meant to quickly check if transformSupergraphToPublicSchema is needed.
Obviously, in most cases when transformSupergraphToPublicSchema is used, a provided schema contains Supergraph spec (join__Graph etc).

Why do I create it then?
We have a special case in GraphQL Hive where we persist the output of transformSupergraphToPublicSchema, but when Apollo Federation adds something new, we might want to run this method again to remove new pieces.

Performance

I used a Supergraph SDL with ~20k LOC and create three copies.
First copy used field(whatever: join__Graph) as an argument somewhere in the middle.
Second copy had scalar join__DirectiveArguments, also in the middle of the file.
Third copy had directive @join__directive definition (yeah yeah, in the middle).

I wrote three versions of containsSupergraphSpec to make sure it has minimal performance footprint.

  1. for-loop over all federation scalars, enums and directives that used sdl.includes("[name") or sdl.includes(" name")
  2. same for-loop but directives where checked first
  3. regex (current implementation).

I ran it 1000 times and I got (average):

  1. 1.56 ms
  2. 0.83 ms
  3. 0.59 ms

Copy link
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@theguild/federation-composition 0.8.2-alpha-20240222093720-e8f2bb3 npm ↗︎ unpkg ↗︎

@kamilkisiela kamilkisiela merged commit de983b0 into main Feb 22, 2024
4 checks passed
@kamilkisiela kamilkisiela deleted the kamil-detect branch February 22, 2024 13:23
kamilkisiela pushed a commit that referenced this pull request Feb 22, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @theguild/[email protected]

### Patch Changes

-   [#46](#46)

[`cfa9950`](cfa9950)
Thanks [@kamilkisiela](https://github.com/kamilkisiela)! - Add
`requiresScopes__Scope` and
    `policy__Policy` to `transformSupergraphToPublicSchema`

-   [#44](#44)

[`de983b0`](de983b0)
Thanks [@kamilkisiela](https://github.com/kamilkisiela)! - Add
containsSupergraphSpec to detect if
    Supergraph related scalars, enums or directives are used

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

1 participant