Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

Adding partytown + gtm #167

Merged
merged 18 commits into from Jan 10, 2022
Merged

Adding partytown + gtm #167

merged 18 commits into from Jan 10, 2022

Conversation

igorbrasileiro
Copy link
Contributor

@igorbrasileiro igorbrasileiro commented Dec 21, 2021

What's the purpose of this pull request?

This PR adds the https://github.com/builderio/partytown and google tag manager.

Pros:

  • Handle google analytics events inside the web worker;
  • Forward events to partytown is synchronous;

How it works?

Tell us the role of the new feature, or component, in its context.

How to test it?

Describe the steps with bullet points. Is there any external reference, link, or example?

References

Spread the knowledge: is this any content you used to create this PR that is worth sharing?

Extra tip: add references to related issues or mention people important to this PR may be good for the documentation and reviewing process

@netlify
Copy link

netlify bot commented Dec 21, 2021

✔️ Deploy Preview for basestore ready!

🔨 Explore the source changes: a55fbe8

🔍 Inspect the deploy log: https://app.netlify.com/sites/basestore/deploys/61dc45723199db000792603b

😎 Browse the preview: https://deploy-preview-167--basestore.netlify.app/bundle-stats

@gatsby-cloud
Copy link

gatsby-cloud bot commented Dec 21, 2021

Gatsby Cloud Build Report

basestore

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 2m

Performance

Lighthouse report

Metric Score
Performance 💚 94
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 93

🔗 View full report

@gatsby-cloud
Copy link

gatsby-cloud bot commented Dec 21, 2021

Gatsby Cloud Build Report

basestore

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 1m

Performance

Lighthouse report

Metric Score
Performance 💚 95
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 93

🔗 View full report

@vtex-sites
Copy link

vtex-sites bot commented Dec 21, 2021

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-167--base.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit 65f84d2

Copy link
Contributor

@icazevedo icazevedo left a comment

Choose a reason for hiding this comment

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

I've left some comments. Besides them, I also think it'd be super interesting for us to have a doc (it can be small) explaining how we're using partytown - maybe going through things you wish you knew when you started this - and the pros and cons of using it, like having no way to debug the events via GTM.

gatsby-node.js Show resolved Hide resolved
gatsby-node.js Show resolved Hide resolved
gatsby-ssr.js Outdated Show resolved Hide resolved
gatsby-node.js Outdated Show resolved Hide resolved
gatsby-ssr.js Outdated
])
} else if (process.env.NODE_ENV === 'development') {
// eslint-disable-next-line
console.log(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(
console.warning(

Copy link
Contributor

@victorhmp victorhmp left a comment

Choose a reason for hiding this comment

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

Awesome! 👏🏻 👏🏻 👏🏻

gatsby-ssr.js Outdated
Comment on lines 48 to 49
console.warn(
'gtmContainerId is not set. Google Tag Manager will not work until you provide a valid container id.'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already great! But how about also adding a link to https://developers.google.com/tag-platform/tag-manager/web#standard_web_page_installation, to make it easier for someone who doesn't know what the container ID is?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even better! Since this is in the store's code, and users will get this from a starter, how about adding something like "Check the analytics section on your store.config.js file."

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
igorbrasileiro and others added 2 commits January 10, 2022 11:39
Co-authored-by: Ícaro Azevedo <icazevedo10@gmail.com>
Co-authored-by: Ícaro Azevedo <icazevedo10@gmail.com>
@igorbrasileiro igorbrasileiro merged commit 65f84d2 into master Jan 10, 2022
@igorbrasileiro igorbrasileiro deleted the feat/partytown-gtm branch January 10, 2022 14:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants