Skip to content

add connect/v0.2 and allow @connect on OBJECT #3228

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 12, 2025
Merged

add connect/v0.2 and allow @connect on OBJECT #3228

merged 3 commits into from
Mar 12, 2025

Conversation

lennyburdette
Copy link
Contributor

@lennyburdette lennyburdette commented Mar 11, 2025 •
edited
Loading

not making this conditional on spec version because that will be enforced by rust code

@lennyburdette lennyburdette requested a review from a team March 11, 2025 15:36
@lennyburdette lennyburdette requested a review from a team as a code owner March 11, 2025 15:36
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 11, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: c644c2d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@apollo/composition Minor
@apollo/federation-internals Minor
apollo-federation-integration-testsuite Minor
@apollo/gateway Minor
@apollo/query-graphs Minor
@apollo/query-planner Minor
@apollo/subgraph Minor

Not sure what this means? Click here to learn what changesets are.

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

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Mar 11, 2025 •
edited
Loading

⚠️ Docs preview not attached to branch

The preview was not built because the PR's base branch next is not in the list of sources.

An Apollo team member can comment one of the following commands to dictate which branch to attach the preview to:

  • !docs set-base-branch version-0.x
  • !docs set-base-branch main

Build ID: 89194014a6481e04a4749f83

@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Mar 11, 2025 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

dylan-apollo
dylan-apollo approved these changes Mar 12, 2025
View reviewed changes
Copy link
Member

@dylan-apollo dylan-apollo left a comment

Choose a reason for hiding this comment

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

LGTM, I think my comments should just become follow-up tickets

.changeset/lucky-pillows-rhyme.md
"@apollo/subgraph": minor
---

Add connect spec v0.2
Copy link
Member

Choose a reason for hiding this comment

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

Should we wait on the changelog entry until we know what all will be included in 0.2? I guess for changeset to bump versions it needs something, so maybe we leave ourselves a task to update this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that's exactly right, i just need something for changeset to use, and we can edit the changelog for the official release. the ticket is CNN-656

composition-js/src/__tests__/connectors.test.ts
Comment on lines +185 to +187
@link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION)
@join__directive(graphs: [WITH_CONNECTORS_V0_1_], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
@join__directive(graphs: [WITH_CONNECTORS_V0_2_], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.2\\", import: [\\"@connect\\", \\"@source\\"]})
Copy link
Member

Choose a reason for hiding this comment

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

Just so I understand, the idea is that the link shows the highest spec number so Router can double-check it's supported? And then the join__directives show which spec is used for each subgraph?

We gotta make sure that when we expand and when we build up connectors for runtime that we're checking the join__directive for spec version and not the link, I get the feeling we're just checking link right now...

Copy link
Member

Choose a reason for hiding this comment

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

I see the comment in merge 👍. Let's either add a follow-up task to audit how we're parsing these or add it to the current open PR on router for @connect on type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the directive in the supergraph is ignored during extract_subgraphs_from_supergraph, so we're good here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we only put the directive in the supergraph so we can add for: EXECUTION and disallow the router from using a supergraph with features it doesn't understand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be more accurate, extract_subgraphs_from_supergraph converts the join__directive link into a normal link in the extracted subgraph. so yeah, we are using the link, but because it's in the subgraph, we know we're using the correct one

Base automatically changed from lb/next-merge-main to next March 12, 2025 20:22
@lennyburdette lennyburdette requested a review from a team as a code owner March 12, 2025 20:22
@lennyburdette
add connect/v0.2 and allow @connect on OBJECT
4419989 
not making this conditional on spec version because that will be enforced by rust code
@lennyburdette
add changeset
49a0542
@lennyburdette
ensure we emit only one link per feature
c644c2d
@lennyburdette lennyburdette merged commit e7e6757 into next Mar 12, 2025
17 checks passed
@lennyburdette lennyburdette deleted the lb/connect-on-types branch March 12, 2025 20:31
@github-actions github-actions bot mentioned this pull request Feb 13, 2025
Merged
lennyburdette pushed a commit that referenced this pull request Mar 12, 2025
@github-actions
release: on branch next (preview) (#3217)
41bdeb7 
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 next, this PR will
be updated.

⚠️⚠️⚠️⚠️⚠️⚠️

`next` is currently in **pre mode** so this branch has prereleases
rather than normal releases. If you want to exit prereleases, run
`changeset pre exit` on `next`.

⚠️⚠️⚠️⚠️⚠️⚠️

# Releases
## @apollo/composition@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

### Patch Changes

- Updated dependencies
\[[`e7e67579908d5cd2fa6fe558228dffe4808cd98d`](e7e6757)]:
    -   @apollo/federation-internals@2.11.0-preview.0
    -   @apollo/query-graphs@2.11.0-preview.0

## @apollo/gateway@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

### Patch Changes

- Updated dependencies
\[[`e7e67579908d5cd2fa6fe558228dffe4808cd98d`](e7e6757)]:
    -   @apollo/composition@2.11.0-preview.0
    -   @apollo/federation-internals@2.11.0-preview.0
    -   @apollo/query-planner@2.11.0-preview.0

## @apollo/federation-internals@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

## @apollo/query-graphs@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

### Patch Changes

- Updated dependencies
\[[`e7e67579908d5cd2fa6fe558228dffe4808cd98d`](e7e6757)]:
    -   @apollo/federation-internals@2.11.0-preview.0

## @apollo/query-planner@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

### Patch Changes

- Updated dependencies
\[[`e7e67579908d5cd2fa6fe558228dffe4808cd98d`](e7e6757)]:
    -   @apollo/federation-internals@2.11.0-preview.0
    -   @apollo/query-graphs@2.11.0-preview.0

## @apollo/subgraph@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

### Patch Changes

- Updated dependencies
\[[`e7e67579908d5cd2fa6fe558228dffe4808cd98d`](e7e6757)]:
    -   @apollo/federation-internals@2.11.0-preview.0

## apollo-federation-integration-testsuite@2.11.0-preview.0

### Minor Changes

- Add connect spec v0.2
([#3228](#3228))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Jun 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylan-apollo dylan-apollo dylan-apollo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lennyburdette @svc-apollo-docs @dylan-apollo