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

[docs] Fix Google Analytics going to the wrong property #6718

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 2, 2022

While I was working on this slide: https://docs.google.com/presentation/d/1wnNqlD5KBIOACRb-mutSiU8fLTqjBlff-2y8j4kDMMc/edit#slide=id.g1626cc333d1_0_16, I saw something very strange on Google Analytics:

Screenshot 2022-11-02 at 23 41 08

https://analytics.google.com/analytics/web/#/report-home/a106598593w159022482p272132536

The hits are sent to the development property instead of the production one.

It turns out that we updated the mono repository #6570 to a version that requires withDocsInfra() to set the env variables requires by the docs. I have copied docs/next.config.js that we have on the next branch and applied it to the master branch. I didn't think about this when I introduced the withDocsInfra() abstraction. I should probably have added some logic so it fails upfront with the env values missing, too late.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation v5.x labels Nov 2, 2022
@oliviertassinari oliviertassinari changed the title [docs] Update nextjs to match docs infra [docs] Fix Google Analytics going to the wrong property Nov 2, 2022
@mui-bot
Copy link

mui-bot commented Nov 2, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 464.4 945.3 548.9 628.36 180.19
Sort 100k rows ms 524.3 1,008.6 754.4 812.4 176.207
Select 100k rows ms 193.7 262.7 213.5 224.94 24.674
Deselect 100k rows ms 134.3 275 208.5 202.98 50.853

Generated by 🚫 dangerJS against 752b10a

eslint: {
ignoreDuringBuilds: true,
},
module.exports = withDocsInfra({
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm I understand it correctly - was the issue here that without withDocsInfra the DEPLOY_ENV variable was missing and development GOOGLE_ANALYTICS_ID was used by default?

Copy link
Member Author

@oliviertassinari oliviertassinari Nov 3, 2022

Choose a reason for hiding this comment

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

@cherniavskii Yes, correct.

I feel that at this point, it's not worth adding more checks as we don't market our docs infra as a standalone open-source project and all the other docs are using withDocsInfra now.

@oliviertassinari oliviertassinari merged commit f12cd74 into mui:master Nov 3, 2022
@oliviertassinari oliviertassinari deleted the update-nextjs-to-match-docs-infra branch November 3, 2022 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation v5.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants