Skip to content

[typescript-resolvers] Refactor to remove NameNode override and simplify federation functions #10377

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

Open
wants to merge 2 commits into
base: federation-fixes
Choose a base branch
from

Conversation

eddeee888
Copy link
Collaborator

@eddeee888 eddeee888 commented Jun 29, 2025

Description

To prepare for next major version, we want to fix a few long-standing type cast issues for typescript-resolvers plugin and federation support:

  • Previously ,NameNode was implemented to change how a node name can be accessed. Instead of node.name.value, it just becomes node.name. However, the type of this node (from graphql package) does not know that this is changed at runtime, so we always need to type case as any as string or as unknown as string to satisfy TypeScript. This results in confusion for contributors, and normally requires runtime debug to understand/correctly use the right value.
    • This PR removes NameNode function to avoid typecasts later
  • Some functions in federation.ts take both AST nodes and types. This adds more complexity as there are more code paths to think about.
    • This PR simplifies and make it more consistent by requiring functions in federation support to use AST nodes.

Type of change

  • Refactor

How Has This Been Tested?

  • Unit test

Copy link

changeset-bot bot commented Jun 29, 2025

⚠️ No Changeset found

Latest commit: c85c666

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@eddeee888 eddeee888 force-pushed the remove-node-name-override branch from 7f026cf to 30134ca Compare June 29, 2025 14:36
@eddeee888 eddeee888 changed the title [typescript-resolvers] Remove NameNode override and refactor relevant references [typescript-resolvers] Refactor to remove NameNode override and simplify federation functions Jun 29, 2025
@eddeee888 eddeee888 marked this pull request as ready for review June 29, 2025 14:57
@eddeee888 eddeee888 requested a review from dotansimha June 29, 2025 14:57
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