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

3.16 - Remove console.log of the beacon script when the debug mode is not enabled #6574

Closed
MathieuLamiot opened this issue Apr 18, 2024 · 3 comments · Fixed by #6595
Closed
Assignees
Labels
effort: [S] 1-2 days of estimated development time

Comments

@MathieuLamiot
Copy link
Contributor

Context
To build the feature and test the beacon script, we relied a lot on console.log() in the beacon implementation to track what was going on there. This can be very valuable for debugging later on and investigations. However, this should not happen for "normal" usage.

Expected behavior
When the script has injected from the WP Rocket plugin while DEBUG mode was off, the beacon script should not output any log.
When the script has injected from the WP Rocket plugin while DEBUG mode was on, the beacon script should output logs.

Acceptance Criteria

  • Enable debug mode (WP_ROCKET_DEBUG = true).
  • Clear the critical images and cache ; enable LCP/ATF.
  • Browse to a page on the website, and check the dev tools: logs from the beacon script should be output in the console.
  • Disable debug mode (WP_ROCKET_DEBUG = true).
  • Clear the critical images and cache ; enable LCP/ATF.
  • Browse to a page on the website, and check the dev tools: no logs from the beacon script should be in the console.

Additional information

  • Maybe we could pass the value of the debug mode to the script when injecting it? The same way we do with a few configurations already. And use this as a condition around our console.log?
  • With this solution, when the beacon is injected while on DEbug mode, then deactivate debug mode, the beacon will still output logs as it can be cached in a page. That's acceptable I think.
@remyperona
Copy link
Contributor

Scope a solution ✅

⚠️ This needs to be done once we have finalized the refactors of the script.

Engine\Media\AboveTheFold\Frontend\Controller

  • in inject_beacon(), add a new key to the $data, with the key debug and value of WP_ROCKET_DEBUG
  • update related tests

assets/js/lcp-beacon.js

  • in places where we currently have console calls, wrap them in condition checking for rocket_lcp_data.debug

Estimate the effort ✅

Effort [S]

@remyperona remyperona added the effort: [S] 1-2 days of estimated development time label Apr 22, 2024
@Miraeld
Copy link
Contributor

Miraeld commented Apr 23, 2024

Looks good to me

@Khadreal Khadreal assigned Khadreal and unassigned Khadreal Apr 23, 2024
@remyperona remyperona self-assigned this Apr 24, 2024
@MathieuLamiot
Copy link
Contributor Author

MathieuLamiot commented Apr 24, 2024

Still pending this PR to be merged: #6555, #6557

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [S] 1-2 days of estimated development time
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants