-
Notifications
You must be signed in to change notification settings - Fork 1
MPDX-9452 - Maintenance: Remove SITE_URL #1781
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ | |
| {process.env.HELPJUICE_ORIGIN && ( | ||
| <script | ||
| dangerouslySetInnerHTML={{ | ||
| __html: `window.helpjuice_account_url = '${process.env.SITE_URL}/api/helpjuice'; | ||
| __html: `window.helpjuice_account_url = window.location.origin + '/api/helpjuice'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
[Suggestion] **Inline-script pattern is correct; informational only.**
The new line emits the literal text Mixed pattern with |
||
| window.helpjuice_contact_us_url = '${process.env.HELPJUICE_ORIGIN}/contact-us'; | ||
| window.helpjuiceSwiftyConfig = { widgetPosition: 'bottomRight' }; | ||
| window.helpjuiceSwiftyUrlMap = {};`, | ||
|
|
@@ -50,7 +50,7 @@ | |
| /> | ||
| )} | ||
| {process.env.HELPJUICE_ORIGIN && ( | ||
| <script src={`${process.env.HELPJUICE_ORIGIN}/swifty.js`} /> | ||
|
Check warning on line 53 in pages/_document.page.tsx
|
||
| )} | ||
| {process.env.DATADOG_CONFIGURED === 'true' && ( | ||
| <Script id="datadog-rum" strategy="afterInteractive"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ const batchNativeHttpLink = new BatchHttpLink({ | |
| }); | ||
|
|
||
| const restProxyHttpLink = new BatchHttpLink({ | ||
| uri: `${process.env.SITE_URL}/api/graphql-rest`, | ||
| uri: `/api/graphql-rest`, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
[Suggestion] **Latent SSR safety risk.** This module is imported by both `client.ts` (browser) and `ssrClient.ts` (Node SSR). The relative URI `/api/graphql-rest` resolves correctly via `window.location.origin` in the browser, but Node's `fetch` rejects relative URLs.
Today no SSR query routes here — verified against The risk is future-only: a developer who adds a REST-proxy-only field to an SSR query will get a confusing const restProxyHttpLink = new BatchHttpLink({
uri: () => {
if (typeof window === 'undefined') {
throw new Error(
'REST-proxy operations cannot run from SSR — add the field to the native GraphQL API or move the query to a client-side hook.',
);
}
return '/api/graphql-rest';
},
batchMax: 25,
batchDebounce: true,
batchInterval: 20,
});Not a blocker — informational only. |
||
| batchMax: 25, | ||
| batchDebounce: true, | ||
| batchInterval: 20, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also update the
amplify.ymlas we no longer need thePREVIEW_URLUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-bizz Is it still needed for
NEXTAUTH_URL=$PREVIEW_URL;inamplify.yml?https://github.com/CruGlobal/mpdx-react/blob/site-url/amplify.yml#L16
We can remove
echo "PREVIEW_URL=$PREVIEW_URL" >> .env;fromamplify.ymlhttps://github.com/CruGlobal/mpdx-react/blob/site-url/amplify.yml#L17
Just not sure about line 16