-
Notifications
You must be signed in to change notification settings - Fork 267
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
Make Analytics component stable #2141
Conversation
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
This comment has been minimized.
This comment has been minimized.
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.
Other than the script hydration stuff, looks great! 🙌
examples/gtm/README.md
Outdated
+ <script nonce={nonce} dangerouslySetInnerHTML={{ | ||
+ __html: `(function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start': | ||
+ new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], | ||
+ j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:'';j.async=true;j.src= | ||
+ 'https://www.googletagmanager.com/gtm.js?id='+i+dl;f.parentNode.insertBefore(j,f); | ||
+ })(window,document,'script','dataLayer','GTM-<YOUR_GTM_ID>');`, | ||
+ }}></script> |
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.
Any reason to not use the Script
component? As is, this will cause hydration warnings, because the nonce
is unavailable in the client. suppressHydrationWarning
needs to be added as a prop: https://github.com/Shopify/hydrogen/blob/main/packages/hydrogen/src/csp/Script.tsx#L10
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.
I was just trying to be as close to as the snippet that GTM supplied - but use <Script>
to surpress hydration warning makes sense
examples/gtm/app/root.tsx
Outdated
@@ -129,18 +129,35 @@ export default function App() { | |||
<meta name="viewport" content="width=device-width,initial-scale=1" /> | |||
<Meta /> | |||
<Links /> | |||
<script nonce={nonce} dangerouslySetInnerHTML={{ |
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.
Either add suppressHydrationWarning
or use the Script
component
Co-authored-by: Bret Little <bret.little@shopify.com>
Co-authored-by: Bret Little <bret.little@shopify.com>
Hey @wizardlyhel! Getting this weird flickering when |
@wizardlyhel - the gtm example on the main branch still says "unstable". |
@napter Ah, I missed the update on the heading text for the GTM readme but the example on the readme are all updated to the latest code. We no longer have the analytics example in the Hydrogen repo. That is now baked into skeleton template and you may follow these docs https://shopify.dev/docs/storefronts/headless/hydrogen/analytics to update your app. |
WHY are these changes introduced?
Making
<Analytics>
stableWHAT is this pull request doing?
<UNSTABLE_Analytics>
stable - rename to<Analytics>
unstable_useAnalytics
stable - rename touseAnalytics
useCustomerPrivacy
so that it will be ready when script is loaded<Analytics.Provider>
works for mock shopcookieDomain
prop for<Analytics.Provider>
so thatuseShopifyCookie
can use the domain overrideanalytics
example to a Goggle Tag Manager exampleHOW to test your changes?
Post-merge steps
Checklist