Rename window.ENV to reduce clashing with other scripts #1520
Conversation
WalkthroughRefactors public environment variable exposure: renames types and global window key, flattens publicEnvVariables shape, updates loader/clientLoader return, inline script injection, and all consumers to use window.NIMBUS_PUBLIC_ENV and top-level VITE_API_URL. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant S as Remix Server
participant U as getPublicEnvVariables
participant L as Layout (HTML)
participant C as Client App
B->>S: Request /
S->>U: getPublicEnvVariables()
U-->>S: { VITE_* keys }
S-->>B: HTML + data.publicEnvVariables
Note over L: Inline script sets<br/>window.NIMBUS_PUBLIC_ENV = data.publicEnvVariables
B->>C: Hydration starts
C->>C: Read NIMBUS_PUBLIC_ENV.VITE_API_URL
C-->>C: Configure Navigation/Dapper/Outlet context
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
In some cases 3rd party scripts, like ads, will try to use such generic name as window.ENV
Oksamies
force-pushed
the
09-04-rename_window.env_to_reduce_clashing_with_other_scripts
branch
from
f598020 to
af1410a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
47-49: Fix user-facing typo in error message“Enviroment” → “Environment”. This surfaces to users.
Apply:
- "Enviroment variables did not load correctly, please hard refresh page" + "Environment variables did not load correctly, please hard refresh the page"apps/cyberstorm-remix/app/root.tsx (1)
124-129: Fix user-facing typo in error messageSame typo here; fix before shipping.
Apply:
- "Enviroment variables did not load correctly, please hard refresh page" + "Environment variables did not load correctly, please hard refresh the page"
🧹 Nitpick comments (3)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
14-16: Simplify the type (avoid double-optional) and consider PascalCase for typesYou can express this more cleanly as a Partial<Record<... , string>>. Also, TypeScript convention is PascalCase for type names.
Apply:
-export type publicEnvVariablesType = Partial<{ - [key in PublicPrefix<publicEnvVariablesKeys>]: string | undefined; -}>; +export type publicEnvVariablesType = Partial< + Record<PublicPrefix<publicEnvVariablesKeys>, string> +>;apps/cyberstorm-remix/app/root.tsx (2)
48-52: Move global Window augmentation to a .d.ts filePlacing global typings in a dedicated
.d.tsavoids reliance on this module’s import order and ensures availability project-wide.Example (create types/global.d.ts):
import type { publicEnvVariablesType } from "cyberstorm/security/publicEnvVariables"; import type { DapperTs } from "@thunderstore/dapper-ts"; declare global { interface Window { NIMBUS_PUBLIC_ENV: publicEnvVariablesType; Dapper: DapperTs; nitroAds?: { createAd: (containerId: string, params: { demo: boolean; format: string; refreshLimit: number; refreshTime: number; renderVisibleOnly: boolean; refreshVisibleOnly: boolean; sizes: string[][]; report: { enabled: boolean; wording: string; position: string }; mediaQuery: string; }) => void; }; } } export {};
93-101: Deduplicate env key lists to avoid driftThe loader and clientLoader repeat the same key array.
Apply this change inside both call sites:
- const publicEnvVariables = getPublicEnvVariables([ - "VITE_SITE_URL", - "VITE_BETA_SITE_URL", - "VITE_API_URL", - "VITE_COOKIE_DOMAIN", - "VITE_AUTH_BASE_URL", - "VITE_AUTH_RETURN_URL", - "VITE_CLIENT_SENTRY_DSN", - ]); + const publicEnvVariables = getPublicEnvVariables([...PUBLIC_ENV_KEYS]);Add once near the imports:
import type { PublicPrefix, publicEnvVariablesKeys } from "cyberstorm/security/publicEnvVariables"; const PUBLIC_ENV_KEYS = [ "VITE_SITE_URL", "VITE_BETA_SITE_URL", "VITE_API_URL", "VITE_COOKIE_DOMAIN", "VITE_AUTH_BASE_URL", "VITE_AUTH_RETURN_URL", "VITE_CLIENT_SENTRY_DSN", ] as const satisfies ReadonlyArray<PublicPrefix<publicEnvVariablesKeys>>;Also applies to: 114-122
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/cyberstorm-remix/app/root.tsx(8 hunks)apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cyberstorm-remix/app/root.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
publicEnvVariablesType(14-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: Generate visual diffs
- GitHub Check: CodeQL
🔇 Additional comments (6)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
20-22: Return typing and local variable typing look consistentThe function signature and
returnedVarstyping now match the new alias.apps/cyberstorm-remix/app/root.tsx (5)
41-42: Type import alignment looks goodImporting
publicEnvVariablesTypehere keeps the window typing consistent.
107-110: Return shape change LGTMUpdated to expose
publicEnvVariablesat the top level.
230-231: Domain prop update LGTMUsing the flattened
VITE_API_URLmatches the new shape.
273-276: Dapper config source LGTM
apiHostnow reads from the flattened env as intended.
283-284: Outlet context domain LGTMConsistent with other updates.
| publicEnvVariablesType, | ||
| } from "cyberstorm/security/publicEnvVariables"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repo-wide verification to catch stragglers after the rename
Scan for old window.ENV and nested .ENV usages; also ensure the new global is referenced where needed. This avoids runtime issues in un-touched routes/components.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Old global or nested shape
rg -n -C2 -S -P '\bwindow\.ENV\b|publicEnvVariables\.ENV\b|ENV\.(VITE_|PUBLIC_)'
# New global usage exists
rg -n -C2 -S -P '\bwindow\.NIMBUS_PUBLIC_ENV\b'
# Catch typos like "Enviroment variables"
rg -n -C1 -S 'Enviroment variables'Length of output: 102
🏁 Script executed:
#!/bin/bash
# Search for old global or nested ENV usages; ignore exit codes to continue scanning
rg -n -C2 -S -P '\bwindow\.ENV\b|publicEnvVariables\.ENV\b|ENV\.(VITE_|PUBLIC_)' || echo "✅ no old ENV usages found"
# Verify new global usage exists
rg -n -C2 -S -P '\bwindow\.NIMBUS_PUBLIC_ENV\b' || echo "⚠️ no new NIMBUS_PUBLIC_ENV references found"
# Catch common typo "Enviroment variables"
rg -n -C1 -S 'Enviroment variables' || echo "✅ no typo matches found"Length of output: 1739
Fix typo in error messages
Change “Enviroment variables did not load correctly…” to “Environment variables did not load correctly…” in:
- apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (line 48)
- apps/cyberstorm-remix/app/root.tsx (line 128)
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts around line
48 and apps/cyberstorm-remix/app/root.tsx around line 128, fix the typo in the
error message by replacing "Enviroment variables did not load correctly…" with
the correct spelling "Environment variables did not load correctly…" (preserve
punctuation and capitalization exactly as in the surrounding messages).
| __html: `window.NIMBUS_PUBLIC_ENV = ${JSON.stringify( | ||
| data.publicEnvVariables | ||
| )}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the global: make it read-only and resilient; consider CSP
Prevent third-party scripts from mutating or overwriting the exposed env by defining it as non-writable + frozen; also don’t overwrite if already set. If you use a CSP, ensure this inline script is nonce’d or hashed.
Apply:
- __html: `window.NIMBUS_PUBLIC_ENV = ${JSON.stringify(
- data.publicEnvVariables
- )}`,
+ __html: `(function(){if(!("NIMBUS_PUBLIC_ENV" in window)){Object.defineProperty(window,"NIMBUS_PUBLIC_ENV",{value:Object.freeze(${JSON.stringify(
+ data.publicEnvVariables
+ )}),writable:false,configurable:false,enumerable:true});}})();`,Optional (CSP): add a nonce to this script tag consistent with your CSP setup.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| __html: `window.NIMBUS_PUBLIC_ENV = ${JSON.stringify( | |
| data.publicEnvVariables | |
| )}`, | |
| __html: `(function(){ | |
| if (!("NIMBUS_PUBLIC_ENV" in window)) { | |
| Object.defineProperty(window, "NIMBUS_PUBLIC_ENV", { | |
| value: Object.freeze(${JSON.stringify( | |
| data.publicEnvVariables | |
| )}), | |
| writable: false, | |
| configurable: false, | |
| enumerable: true | |
| }); | |
| } | |
| })();`, |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/root.tsx around lines 220-222, the inline script
currently assigns window.NIMBUS_PUBLIC_ENV directly which allows mutation or
overwrite; change it to only set the value if undefined, then Object.freeze the
object and use Object.defineProperty to make the global
non-writable/configurable so third-party scripts cannot mutate it; ensure the
script generation includes any CSP nonce or hash required by your CSP setup so
the inline script is allowed.
| vars.forEach((envVar) => { | ||
| if (envVar.startsWith("VITE_") && envVar in window.ENV) { | ||
| returnedVars[envVar] = window.ENV[envVar]; | ||
| if (envVar.startsWith("VITE_") && envVar in window.NIMBUS_PUBLIC_ENV) { | ||
| returnedVars[envVar] = window.NIMBUS_PUBLIC_ENV[envVar]; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Defensive access to window.NIMBUS_PUBLIC_ENV to prevent runtime errors
If the inline script fails to run (e.g., CSP), window.NIMBUS_PUBLIC_ENV will be undefined and in will throw. Also prefer own-property checks.
Apply:
- } else {
- vars.forEach((envVar) => {
- if (envVar.startsWith("VITE_") && envVar in window.NIMBUS_PUBLIC_ENV) {
- returnedVars[envVar] = window.NIMBUS_PUBLIC_ENV[envVar];
- }
- });
- }
+ } else {
+ const publicEnv =
+ typeof window !== "undefined" && (window as any).NIMBUS_PUBLIC_ENV
+ ? ((window as any).NIMBUS_PUBLIC_ENV as Record<string, string | undefined>)
+ : {};
+ vars.forEach((envVar) => {
+ if (
+ envVar.startsWith("VITE_") &&
+ Object.prototype.hasOwnProperty.call(publicEnv, envVar)
+ ) {
+ returnedVars[envVar] = publicEnv[envVar];
+ }
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| vars.forEach((envVar) => { | |
| if (envVar.startsWith("VITE_") && envVar in window.ENV) { | |
| returnedVars[envVar] = window.ENV[envVar]; | |
| if (envVar.startsWith("VITE_") && envVar in window.NIMBUS_PUBLIC_ENV) { | |
| returnedVars[envVar] = window.NIMBUS_PUBLIC_ENV[envVar]; | |
| } | |
| }); | |
| } else { | |
| const publicEnv = | |
| typeof window !== "undefined" && (window as any).NIMBUS_PUBLIC_ENV | |
| ? ((window as any).NIMBUS_PUBLIC_ENV as Record<string, string | undefined>) | |
| : {}; | |
| vars.forEach((envVar) => { | |
| if ( | |
| envVar.startsWith("VITE_") && | |
| Object.prototype.hasOwnProperty.call(publicEnv, envVar) | |
| ) { | |
| returnedVars[envVar] = publicEnv[envVar]; | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts around lines
29 to 33, the code uses the `in` operator against `window.NIMBUS_PUBLIC_ENV`
which will throw if that global is undefined (e.g., CSP blocked the inline
script); change the logic to first defensively check that `window` and
`window.NIMBUS_PUBLIC_ENV` are defined (truthy) before iterating, and when
checking membership use an own-property check (e.g.,
Object.prototype.hasOwnProperty.call(window.NIMBUS_PUBLIC_ENV, envVar)) so you
avoid prototype-inherited keys and runtime errors; if the global is missing,
skip adding vars or fall back to an empty object.
In some cases 3rd party scripts, like ads, will try to use such generic
name as window.ENV
Summary by CodeRabbit