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.

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

feat: allow overloading of query from snapwp configs #83

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

feat: allow overloading of query from snapwp configs #83

wants to merge 3 commits into from
+51 −21

Conversation

SH4LIN
Copy link
Collaborator

@SH4LIN SH4LIN commented Feb 20, 2025 •
edited
Loading

What

  • This PR will allow developers to pass their generated query document to the config in order to execute their own query instead of our own.

Caution

  • This will allow developers to pass their own query, but we can't enforce any restrictions on its format. There is a high chance that if a developer does not structure the query as expected, the frontend will break.

Related Issue(s):

How

  • Created a config property to which developers can pass their query document which will be used to execute the graphql query

Testing Instructions

  • Create a new query inside the /examples/nextjs/starter/src
  • Execute npm run codegen
  • Pass the generated document from the config.

Checklist

  • I have read the Contribution Guidelines.
  • My code is tested to the best of my abilities.
  • My code passes all lints (ESLint, tsc, prettier etc.).
  • My code has detailed inline documentation.
  • I have added unit tests to verify the code works as intended.
  • I have updated the project documentation as needed.
  • I have added a changeset for this PR using npm run changeset.

@SH4LIN SH4LIN requested review from ayushnirwal and justlevine and removed request for ayushnirwal February 20, 2025 11:19
justlevine
justlevine reviewed Feb 21, 2025
View reviewed changes
packages/types/src/queries/global-styles.ts
@@ -0,0 +1,4 @@
import type { TypedDocumentNode } from '@apollo/client';

// Until we figure out generic types for GraphQL queries, we'll use `any` for now.
Copy link
Collaborator

@justlevine justlevine Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why any vs unknown? Beyond my personal hatred of any, what are the implications of each ?

@justlevine justlevine added status: hold ⏸️ Needs other items to be resolved before work can continue and removed status: hold ⚠️ status: hold ⏸️ Needs other items to be resolved before work can continue labels Feb 23, 2025
@ayushnirwal
Copy link
Collaborator

Is this on hold?

@justlevine
Copy link
Collaborator

justlevine commented Mar 10, 2025 •
edited
Loading

Is this on hold?

Not anymore (was bumped only from v0.1.0), but we did want to revisit it in context of more general changes we need to make to the QueryEngine

@changeset-bot changeset-bot
Copy link

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

⚠️ No Changeset found

Latest commit: c0e18fb

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

@justlevine justlevine added the stale? 🥖 This might be out of date. label Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justlevine justlevine justlevine left review comments

@ayushnirwal ayushnirwal Awaiting requested review from ayushnirwal

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
stale? 🥖 This might be out of date.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SH4LIN @ayushnirwal @justlevine