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

fix: directives name clashes with apollo federation for link #2886

Merged

Conversation

meskill
Copy link
Contributor

@meskill meskill commented Sep 23, 2024

Summary:
Briefly describe the changes made in this PR.

Issue Reference(s):
Fixes #... (Replace "..." with the issue number)

Build & Testing:

  • I ran cargo test successfully.
  • I have run ./lint.sh --mode=fix to fix all linting issues raised by ./lint.sh --mode=check.

Checklist:

  • I have added relevant unit & integration tests.
  • I have updated the documentation accordingly.
  • I have performed a self-review of my code.
  • PR follows the naming convention of <type>(<optional scope>): <title>

src/core/sdl.rs Outdated
#[derive(Default)]
pub struct SdlPrinter {
pub federation_compatibility: bool,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting approach where we by default add federation specific items if the flag is set but then we are scattering this sub-graph specific logic everywhere in our codebase. My recommendation is to add a top-level definitions field in Config where we can add any additional definitions, like in this case - @link these additional definitions aren't consumed by Tailcall but are outputed as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not exactly, the approach is not to output the Tailcall directives that conflicts with federation directives. To make the sdl output federation compatible we need to include apollo spec with federation's @link and by doing so we can't now leave tailcall's usage and definitions of @link

@tusharmath tusharmath merged commit 866bc3d into feat/federation-entity-resolver Sep 24, 2024
7 checks passed
@tusharmath tusharmath deleted the fix/federation-entity-link branch September 24, 2024 14:13
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