kayx23 commented on code in PR #2136:
URL: https://github.com/apache/apisix-website/pull/2136#discussion_r4259352362
##########
next/src/components/Analytics.astro:
##########
@@ -0,0 +1,17 @@
+<!-- Keep production measurement independent of the build/preview environment.
-->
+<script is:inline>
+ (() => {
+ if (window.location.hostname !== 'apisix.apache.org') return;
+
+ window.dataLayer = window.dataLayer || [];
+ window.gtag = window.gtag || function () {
window.dataLayer.push(arguments); };
+ window.gtag('js', new Date());
+ // Scope cookies to this project rather than the shared apache.org domain.
+ window.gtag('config', 'G-WQLBQL6GY3', { cookie_domain: 'apisix.apache.org'
});
+
+ const tag = document.createElement('script');
+ tag.async = true;
+ tag.src = 'https://www.googletagmanager.com/gtag/js?id=G-WQLBQL6GY3';
+ document.head.appendChild(tag);
Review Comment:
No change: the queued initialization and configuration calls are established
before the dynamically created external script can execute. The async resource
cannot run until it has been appended, so moving the append operation ahead of
queue initialization would not improve determinism and would introduce a
theoretical race instead.
##########
next/scripts/analytics.test.mjs:
##########
@@ -0,0 +1,48 @@
+import assert from 'node:assert/strict';
+import { readFile } from 'node:fs/promises';
+import test from 'node:test';
+import vm from 'node:vm';
+
+const source = await readFile(new URL('../src/components/Analytics.astro',
import.meta.url), 'utf8');
+const script = source.match(/<script is:inline>([\s\S]*?)<\/script>/)[1];
+
+function visit(hostname, existing = {}) {
+ const loaded = [];
+ const window = { location: { hostname }, ...existing };
+ const document = {
+ createElement: () => ({}),
Review Comment:
No change: this stub intentionally captures the created element, and the
test already asserts the two script properties the implementation relies on:
the async flag and the exact source URL. Adding unused DOM methods would not
catch a current behavior gap.
##########
next/src/components/Analytics.astro:
##########
@@ -0,0 +1,17 @@
+<!-- Keep production measurement independent of the build/preview environment.
-->
+<script is:inline>
+ (() => {
+ if (window.location.hostname !== 'apisix.apache.org') return;
+
+ window.dataLayer = window.dataLayer || [];
+ window.gtag = window.gtag || function () {
window.dataLayer.push(arguments); };
+ window.gtag('js', new Date());
+ // Scope cookies to this project rather than the shared apache.org domain.
+ window.gtag('config', 'G-WQLBQL6GY3', { cookie_domain: 'apisix.apache.org'
});
+
+ const tag = document.createElement('script');
+ tag.async = true;
+ tag.src = 'https://www.googletagmanager.com/gtag/js?id=G-WQLBQL6GY3';
+ document.head.appendChild(tag);
Review Comment:
The new inline bootstrap is emitted on every page even when the hostname
guard prevents GA from loading. Both `Astro main pages` and the full `build`
now fail because `tests/e2e/ecosystem-pages.spec.mjs:166` still requires zero
non-JSON-LD scripts on the Redis page (desktop and mobile). Please update that
regression test to allow/assert this intentional hostname-gated analytics
bootstrap, while preserving the no-framework-hydration boundary, so the PR does
not merge with two failing required workflows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
