Skip to content

fix: use SITE_URL for GraphQL endpoint instead of HOME_URL #154

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

Merged
merged 3 commits into from
Apr 11, 2025

Conversation

Swanand01
Copy link
Contributor

@Swanand01 Swanand01 commented Apr 10, 2025

What

This PR uses SITE_URL for GraphQL endpoint instead of HOME_URL

Why

In a bedrock setup, the graphql server is at host:port/wp/graphql and host:port/wp is the WP SITE_URL whereas often the HOME_URL will be just host:port

Related Issue(s):

#149

How

Updated instances of generateGraphQLUrl() - passed SITE_URL instead of HOME_URL, per WPGraphQL’s logic.

Testing Instructions

Screenshots

Additional Info

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.

Copy link

changeset-bot bot commented Apr 10, 2025

🦋 Changeset detected

Latest commit: d2a2a63

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

This PR includes changesets to release 2 packages
Name Type
@snapwp/query Patch
@snapwp/core Patch

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

@coveralls
Copy link

coveralls commented Apr 10, 2025

Pull Request Test Coverage Report for Build 14394092408

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 53.768%

Totals Coverage Status
Change from base Build 14368078547: 0.0%
Covered Lines: 399
Relevant Lines: 658

💛 - Coveralls

@Swanand01 Swanand01 requested a review from justlevine April 10, 2025 18:17
@@ -374,7 +374,8 @@ class SnapWPConfigManager {
*/
static getGraphqlUrl(): string {
return generateGraphqlUrl(
SnapWPConfigManager.getConfig().wpHomeUrl,
SnapWPConfigManager.getConfig().wpSiteUrl ||
SnapWPConfigManager.getConfig().wpHomeUrl,
Copy link
Collaborator

Choose a reason for hiding this comment

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

wpSiteUrl already falls back to wpHomeUrl.

Suggested change
SnapWPConfigManager.getConfig().wpHomeUrl,
SnapWPConfigManager.getConfig().wpSiteUrl,

@@ -13,7 +13,8 @@ const config: CodegenConfig = {
schema: process.env.GRAPHQL_SCHEMA_FILE ?? [
{
[ generateGraphqlUrl(
process.env.NEXT_PUBLIC_WP_HOME_URL,
process.env.NEXT_PUBLIC_WP_SITE_URL ||
process.env.NEXT_PUBLIC_WP_HOME_URL,
process.env.NEXT_PUBLIC_GRAPHQL_ENDPOINT
) ]: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This whole things is getting too messy, and not just because I hate the generate() function.

"@snapwp/core": patch
---

feat: use SITE_URL for GraphQL endpoint instead of HOME_URL
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a fix not a feature.

Suggested change
feat: use SITE_URL for GraphQL endpoint instead of HOME_URL
fix: use NEXT_PUBLIC_SITE_URL for GraphQL endpoint when available.

@justlevine justlevine changed the title feat: use SITE_URL for GraphQL endpoint instead of HOME_URL fix: use SITE_URL for GraphQL endpoint instead of HOME_URL Apr 11, 2025
@justlevine justlevine merged commit 22ed5d2 into rtCamp:develop Apr 11, 2025
19 checks passed
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.

3 participants