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

Parameterized graphql endpoint and caching #96

Merged
merged 12 commits into from
Dec 7, 2023

Conversation

jihuang-adobe
Copy link
Contributor

Description

Instead of hardcoding it in usePersistedQueries.js, graphql endpoint namespace is now part of .env.
Graphql caching can be configured in .env.

Related Issue

#95

Motivation and Context

Both customers with AEM Guides WKND installed and Adobe internal with Reference Demo installed can use the code hosted on code sandbox, simply change the graphql endpoint namespace to enable the sample hosted app to pull content from their environment.
https://experienceleague.adobe.com/landing/experience-manager/headless/developer/code/basic-react-app.html?lang=en

How Has This Been Tested?

tested locally and tested on online hosted sandbox
https://codesandbox.io/p/sandbox/wknd-react-app-forked-gfrtlf

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Contributor

@davidjgonzalez davidjgonzalez left a comment

Choose a reason for hiding this comment

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

@jihuang-adobe can you take a look at these changes.

  1. Changes the wknd-shared to be the default as afiak most people (outside Adobe SC) use wknd (and not RDA)
  2. Changed semantics of the Cache toggle, since what youre really doing is disabling the cache w/ its addition (it caches by default). This also helps communicate that disabling is the "outlier" case. I also added a number of comments to re-emphasize disabling cache should not be done on Production.

react-app/.env.development Outdated Show resolved Hide resolved
react-app/.env.development Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
react-app/src/api/usePersistedQueries.js Outdated Show resolved Hide resolved
Copy link
Contributor

@davidjgonzalez davidjgonzalez left a comment

Choose a reason for hiding this comment

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

LGTM - any more changes you have @jihuang-adobe ?

@jihuang-adobe
Copy link
Contributor Author

OMG, LGTM is good for me. Thanks

@davidjgonzalez davidjgonzalez merged commit 44edc7e into adobe:main Dec 7, 2023
2 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.

2 participants