Copilot commented on code in PR #2136:
URL: https://github.com/apache/apisix-website/pull/2136#discussion_r4257105276
##########
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];
Review Comment:
If the `<script is:inline>` block is missing or the regex doesn't match,
`source.match(...)` returns `null` and indexing `[1]` will throw a non-obvious
TypeError. Make the failure explicit by asserting the match exists (or throwing
a clear error) before accessing the capture group so test failures are
diagnosable.
##########
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 GA4 loader appends the external gtag script *after* queueing
`gtag('js'...)` and `gtag('config'...)`. While GA often tolerates this, the
recommended pattern is to load the `gtag/js` script first (with `async`), then
queue initialization/config so timing is deterministic (especially around
consent/default settings and early pageview behavior). Consider inserting the
script tag before the first `gtag(...)` calls, or at least appending the script
before calling `gtag('config', ...)`.
##########
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:
The test stub for `document.createElement` returns a plain object, which
means the production code could start relying on common script element
properties (`type`, `referrerPolicy`, `setAttribute`, etc.) and the test would
not catch missing behavior. Consider returning a minimal \"script-like\" object
(e.g., with `setAttribute` and `type` fields) or validating that the created
element is specifically used as a script (e.g., by checking it contains the
expected `src`/`async` after the script runs). This keeps the test robust if
the analytics implementation evolves.
--
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]
