Skip to content

[website] Fix GA events going to development#899

Merged
bharatkashyap merged 5 commits intomasterfrom
check-ga-bug
Sep 6, 2022
Merged

[website] Fix GA events going to development#899
bharatkashyap merged 5 commits intomasterfrom
check-ga-bug

Conversation

@bharatkashyap
Copy link
Collaborator

@bharatkashyap bharatkashyap commented Sep 5, 2022

  • This is to fix Google Analytics issues going to the "development" instance of Google Analytics

See https://github.com/mui/material-ui/blob/master/docs/pages/_document.js#L34 and
Screenshot 2022-09-06 at 3 07 20 AM

@render
Copy link

render bot commented Sep 5, 2022

@oliviertassinari oliviertassinari requested a deployment to check-ga-bug - toolpad-db PR #899 September 5, 2022 21:42 — with Render Abandoned
@bharatkashyap
Copy link
Collaborator Author

bharatkashyap commented Sep 6, 2022

Value of

process.env.PULL_REQUEST,
  typeof process.env.PULL_REQUEST,
  JSON.stringify(process.env.PULL_REQUEST),

in the deploy preview build:

Screenshot 2022-09-06 at 10 36 41 AM

@Janpot
Copy link
Member

Janpot commented Sep 6, 2022

Ok, I understand now. During SSR it always uses the environment variables available on the server. netlify passes PULL_REQUEST=false during production build. We are not converting process.env.PULL_REQUEST to a boolean in next.config.js (like the monorepo does). So it gets interpolated as "false", which is a truthy value, thus the Toolpad docs thinks it's always building a PR preview.

Solution, either:

  • do PULL_REQUEST: process.env.PULL_REQUEST === 'true' like in the monorepo. (This is kind of brittle, it makes the value of PULL_REQUEST a string in some places, and a boolean in other. We're also relying on a variable that is defined by the CI provider)
  • define process.env.GA_ID and pass the correct value in CI

@bharatkashyap
Copy link
Collaborator Author

Ok, I understand now. During SSR it always uses the environment variables available on the server. netlify passes PULL_REQUEST=false during production build. We are not converting process.env.PULL_REQUEST to a boolean in next.config.js (like the monorepo does). So it gets interpolated as "false", which is a truthy value, thus the Toolpad docs thinks it's always building a PR preview.

Solution, either:

  • do PULL_REQUEST: process.env.PULL_REQUEST === 'true' like in the monorepo. (This is kind of brittle, it makes the value of PULL_REQUEST a string in some places, and a boolean in other. We're also relying on a variable that is defined by the CI provider)
  • just pass process.env.GA_ID and pass the correct value in CI

Let's go ahead and take the first route for now so that we can defer to the existing way of doing things in the immediate term

@bharatkashyap bharatkashyap changed the title [website] Figure out the GA development/prod issue [website] Fix GA events going to development Sep 6, 2022
@bharatkashyap bharatkashyap merged commit b6f0da3 into master Sep 6, 2022
@bharatkashyap bharatkashyap deleted the check-ga-bug branch September 6, 2022 12:16
@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation. website Pages that are not documentation-related, marketing-focused. labels Sep 11, 2022
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 11, 2022

mui/material-ui#34259 should prevent this bug from happening again, making Toolpad less coupled with the docs infra.

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. website Pages that are not documentation-related, marketing-focused.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants