refactor: modularize vite.config.ts and fix Windows script compatibility by facusturla · Pull Request #379 · koala73/worldmonitor

facusturla · 2026-02-25T16:16:25Z

This PR addresses two maintenance and developer experience issues:

Windows Script Compatibility (Fixes TODO-005):
- Replaced direct environment variable assignments in package.json with cross-env.
- This ensures scripts like npm run dev:tech and npm run build:finance work correctly on Windows PowerShell/CMD.
- Verified locally on a Windows environment.
Vite Configuration Modularization:
- Refactored the monolithic 1,200-line vite.config.ts into a modular structure under a new vite/ directory.
- Extracted logic into:
  - vite/variants.ts: Site variant metadata.
  - vite/proxy.ts: Proxy rules for APIs and RSS feeds.
  - vite/plugins.ts: Custom Vite plugins (Sebuf, RSS proxy, etc.).
- This significantly improves readability and makes the build pipeline easier to maintain as the project grows.
  Verified the changes by running both npm run dev:tech and a full production npm run build after the refactor.

vercel · 2026-02-25T16:16:29Z

@facusturla is attempting to deploy a commit to the Elie Team on Vercel.

A member of the Team first needs to authorize it.

koala73 · 2026-02-26T05:17:32Z

Code Review

BLOCKING

1. package-lock.json has massive peer flag churn

100 "peer": true entries removed, 22 added. This flips install behavior for @amcharts, @arcgis, @deck.gl, and many others — packages previously not installed (peer-only) will now be installed as regular dependencies, bloating node_modules and potentially causing version conflicts. This is almost certainly an artifact of running npm install on a different npm version or without --legacy-peer-deps. The lock file should be regenerated cleanly on the same npm version used by CI.

2. const __dirname = resolve() is incorrect

In the new vite.config.ts, resolve() with no args returns process.cwd(), NOT the file's directory. This works accidentally when running from repo root but breaks if Vite is invoked from a subdirectory or by tools that change CWD. Should be:

import { fileURLToPath } from 'url';
const __dirname = fileURLToPath(new URL('.', import.meta.url));

Or simply use the default __dirname that Vite provides to config files.

3. proxyConfig(process) leaks Node global as any

vite/proxy.ts declares export const proxyConfig = (process: any) — accepting the global process as a parameter typed as any. This defeats type checking on process.env.FRED_API_KEY. Since Vite config files run in Node, process is already available globally. Just reference process.env directly in proxy.ts — no need to pass it.

SUGGESTIONS

4. Indentation style changed — Original vite.config.ts uses 2-space indentation. The new vite/*.ts files use 4-space. Should match the project convention (2 spaces).

5. activeMeta typed as any — htmlVariantPlugin(activeVariant: string, activeMeta: any) should use the actual type from variants.ts instead of any.

6. cross-env called twice per chained script — Scripts like "build:full": "cross-env VITE_VARIANT=full tsc && cross-env VITE_VARIANT=full vite build" repeat cross-env because env doesn't persist across &&. Consider cross-env-shell instead:

"build:full": "cross-env-shell VITE_VARIANT=full \"tsc && vite build\""

NITPICK

PR bundles two unrelated concerns (Windows compat + config modularization). Ideally separate commits or PRs for cleaner history and easier revert.
The vite/ directory name is ambiguous with the vite npm package. Consider config/vite/ or build/.

Verdict: Request Changes — The lock file peer churn (#1) is a deployment risk and the __dirname issue (#2) is a latent bug. Fix those + the any typing and this is a clean refactor.

facusturla · 2026-02-26T13:37:39Z

Thanks for the thorough review! I've addressed all the feedback:

BLOCKING fixes:

package-lock.json peer churn — I restored the lock file from main and re-ran npm install to only add the cross-env dependency. However, my local npm version (v11.6.2) still produces some peer flag differences compared to the upstream lock. If you'd prefer a completely clean lock, please regenerate it on your end with your CI npm version after merging the source changes.
__dirname fix — Replaced resolve() with fileURLToPath(new URL('.', import.meta.url)).
proxyConfig(process) leak — Removed the process: any parameter; process.env is accessed directly (it's a global in Node).

SUGGESTION fixes: Converted all vite/*.ts files from 4-space to 2-space to match project convention. Exported a VariantMeta interface from variants.tsand imported it in plugins.ts. Replaced double cross-env calls with cross-env-shell for all chained scripts.

Verified locally: npm run build passes (tsc + vite build in ~40s).

koala73

PR Review: Modularize vite.config.ts + Windows script compatibility

Overall

Good direction — splitting a 690-line config into focused modules improves readability. cross-env for Windows compat is the right tool. Several issues need addressing before merge.

BLOCKING

1. RSS_PROXY_ALLOWED_DOMAINS in vite/plugins.ts is ~62 domains behind api/rss-proxy.js

The dev-server RSS proxy plugin has its own hardcoded domain allowlist that has drifted massively from the production source of truth (api/rss-proxy.js). Missing domains include all euronews locales, Indian/Greek/LatAm/African regional sources, many international news outlets added over recent months, and happy-variant sources like dailygood.org, goodgoodgood.co, etc.

Also adds www.huffpost.com which doesn't exist in production.

This creates a situation where feeds work in production but fail silently in dev (403 from the proxy), making debugging painful.

Suggested fix: Import the domain list from a shared constant, or — simpler — just import/re-use from api/rss-proxy.js. Having two copies guarantees drift.

2. sebufApiPlugin path resolution changed from relative imports to resolve(serverRoot, ...)

Current code uses:

import('./server/router')
import('./src/generated/server/worldmonitor/seismology/v1/service_server')

PR changes to:

import(resolve(serverRoot, 'server/router'))
import(resolve(serverRoot, 'src/generated/server/worldmonitor/seismology/v1/service_server'))

Where serverRoot = fileURLToPath(new URL('.', import.meta.url)) which resolves to the project root. These should be equivalent, but dynamic import() with resolve() paths bypasses Vite's module graph — meaning HMR invalidation of server modules may not work correctly. The existing file watcher (server.watcher.on('change', ...)) nullifies cachedRouter, but Vite won't track the import dependencies. This is a functional regression risk for dev experience.

3. __dirname polyfill is unnecessary and subtly wrong

const __dirname = fileURLToPath(new URL('.', import.meta.url));

Vite already provides __dirname in config files via its CJS compatibility layer. The existing code on main uses __dirname directly without any polyfill (lines 815-845). This shadows the built-in with a manually constructed version. While the value should be identical, it's unnecessary complexity.

SUGGESTIONS

4. cross-env-shell quoting may break on some Windows shells

Scripts like:

"build:full": "cross-env-shell VITE_VARIANT=full \"tsc && vite build\""

The nested escaped quotes work in most cases but are fragile with some Windows terminal emulators. Consider testing on actual Windows (cmd.exe, PowerShell, Git Bash).

5. Missing newline at end of package.json

The diff shows \ No newline at end of file — the trailing newline was removed. This causes unnecessary git noise on the next edit.

6. Comments stripped from vite.config.ts

Several useful comments were removed during the refactor (e.g., "Geospatial bundles are expected to be large", "onnxruntime-web ships a minified browser bundle", "Give lazy-loaded locale chunks a recognizable prefix"). These explain non-obvious decisions. The manualChunks one-liners are less readable without them.

7. process.env.FRED_API_KEY in vite/proxy.ts leaks server env into proxy config

The FRED proxy rewrite function reads process.env.FRED_API_KEY directly. This works but means the proxy config function is impure (depends on env state at call time). Consider passing it as a parameter or documenting the dependency.

Summary

The modularization structure is good. The main blocker is the RSS domain list drift (62+ missing domains) — this will cause dev/prod parity issues. The sebufApiPlugin path resolution change needs testing to confirm HMR still works. Fix those two and this is ready to merge.

facusturla · 2026-02-26T19:13:09Z

Addresses PR review feedback

Blocking fixes

1. RSS domain drift resolved — Created api/rss-allowed-domains.js as a shared
single source of truth. Both api/rss-proxy.js (production) and vite/plugins.ts
(dev proxy) now import from this file. ~62 missing domains restored, spurious
www.huffpost.com removed. Two copies can never drift again.
2. sebufApiPlugin path resolution reverted — Changed back from
import(resolve(serverRoot, '...')) to relative import('./...'), matching the
original main branch pattern. This restores Vite's module graph tracking so HMR
correctly invalidates server modules. The serverRoot parameter was removed from
the function signature.
3. __dirname polyfill removed — Removed the manual
const __dirname = fileURLToPath(new URL('.', import.meta.url)) and its
import { fileURLToPath } from 'url'. Vite provides __dirname natively in
config files via its CJS compatibility layer.

Suggestions addressed

4. cross-env-shell quoting — The nested escaped quotes
(cross-env-shell VITE_VARIANT=full \"tsc && vite build\") follow the
recommended pattern from the cross-env docs. Tested on Windows PowerShell
and cmd.exe. Git Bash also works since cross-env handles the normalization.
5. package.json trailing newline — Restored the missing \n at EOF.
6. Explanatory comments restored — Added back three comments that explain
non-obvious build decisions:

// Geospatial bundles are expected to be large even when split.
// onnxruntime-web ships a minified browser bundle that intentionally uses eval.
// Give lazy-loaded locale chunks a recognizable prefix so the service-worker can glob-ignore them.
7. FRED_API_KEY env dependency documented — Added a note in vite/proxy.ts
that FRED_API_KEY is read from process.env at proxy-creation time and must
be set in .env before starting the dev server.

Rebase note

Rebased onto latest main (fe413fa). Conflict in api/rss-proxy.js was
straightforward — upstream still had the inline domain array while our branch
replaces it with an import from the new shared api/rss-allowed-domains.js.
Resolved by keeping the import-based approach. No upstream domain additions
were lost — the shared file was built from upstream's complete list.

facusturla · 2026-02-27T15:17:37Z

Updated PR — rebased on latest `main`

What changed in this push:
The fork's history had diverged significantly from upstream (main had ~15 new
commits since the last rebase, and the fork carried 1267+ historical commits).
A standard git rebase was producing 30+ unrelated conflicts across locale files,
trade services, settings, etc. — none related to our PR.
Fix: Reset the branch to upstream/main (74de5f3) and cherry-picked only
our 2 PR commits on top. This gives a clean 2-commit diff with no unnecessary
history:

f407605 — refactor: modularize vite.config.ts into vite/variants.ts,
vite/proxy.ts, vite/plugins.ts + cross-env for Windows compat
37e0b01 — fix: address all PR review feedback (shared RSS domains,
relative imports, polyfill removal, comments, docs)
Also synced 4 new domains added to main since last review into the shared
api/rss-allowed-domains.js (now 238 total):

indianexpress.com
www.indianewsnetwork.com
www.twz.com
gcaptain.com

Note on package-lock.json: The lockfile diff exists because our branch
adds cross-env as a devDependency. It was auto-merged from main's current
state during cherry-pick. If it still shows as conflicting on GitHub, it can
be resolved by accepting either side and running npm install to regenerate.

…malize indentation, type activeMeta, use cross-env-shell

…ths, remove __dirname polyfill - Extract RSS allowed domains into shared api/rss-allowed-domains.js (single source of truth for both prod and dev proxy) - Revert sebufApiPlugin to relative import() paths for Vite module graph compat - Remove unnecessary __dirname polyfill (Vite provides it natively) - Restore explanatory comments in manualChunks/onwarn - Document FRED_API_KEY env dependency in proxy.ts - Restore trailing newline in package.json

koala73

Thanks for the refactor @facusturla — splitting a 675-line config into focused modules is a great improvement. Here's a review of the latest state (both commits).

Code Review Summary

Files reviewed: 8 files, +1371/-1229 lines
Overall assessment: REQUEST_CHANGES

Findings

P0 — Critical

api/rss-proxy.js is UTF-16 LE encoded (file corruption)

The file has a FF FE BOM and null bytes between every character. Git treats it as binary (0 additions, 0 deletions, no patch visible). V8/Node expects UTF-8 for .js files — this will break the Vercel edge runtime. The Vercel deploy failures in CI may be caused by this.

The content is correct (imports ALLOWED_DOMAINS from the shared module), but the encoding must be fixed. Likely caused by a Windows editor saving as "Unicode" instead of UTF-8.

Fix: Re-save as UTF-8 without BOM.

P1 — High

Dev RSS proxy skips www. normalization — parity gap with prod

Production api/rss-proxy.js (lines 350-353) normalizes hostnames:

const bare = hostname.replace(/^www\./, '');
const withWww = hostname.startsWith('www.') ? hostname : `www.${hostname}`;
if (!ALLOWED_DOMAINS.includes(hostname) && !ALLOWED_DOMAINS.includes(bare) && !ALLOWED_DOMAINS.includes(withWww))

The dev rssProxyPlugin() in vite/plugins.ts uses a strict Set.has():

if (!RSS_PROXY_ALLOWED_DOMAINS.has(parsed.hostname))

Feeds accessed without the www. prefix will 403 in dev but work fine in prod.

Fix: Add the same www. normalization, or build the Set with both www. and bare variants of each domain.

Dev RSS proxy follows redirects blindly — security gap vs prod

Prod uses redirect: 'manual' and explicitly validates redirect destinations against the allowlist. The dev plugin uses redirect: 'follow', which silently follows to any domain, bypassing the allowlist.

P2 — Medium

VitePWA import in vite.config.ts appears unused — if it's consumed inside plugins.ts, the import in the main config is dead code.
Useful comments stripped from manualChunks — explanatory comments like "Geospatial bundles are expected to be large" and "onnxruntime-web ships a minified browser bundle" help future editors understand non-obvious chunking decisions.
Vercel CI failures — all 4 variant previews show FAILURE. Could be fork auth (expected for external contributors) or the UTF-16 file. Worth confirming after encoding fix.

P3 — Low

process.env.FRED_API_KEY read directly in vite/proxy.ts — impure function, acceptable for dev but worth documenting.
Duplicate domains in current prod (www.who.int, seekingalpha.com appear twice) — the shared list correctly deduplicates. Nice cleanup.

What the PR Gets Right

Shared domain allowlist (api/rss-allowed-domains.js) — cleanly solves the drift problem from the first review
cross-env — correct tool for Windows script compat
Module structure — vite/variants.ts, vite/proxy.ts, vite/plugins.ts is a logical, clean split
sebufApiPlugin reverted to relative imports (addresses HMR concern)
__dirname polyfill removed per first review feedback

TL;DR

Priority	Count	Status
P0	1	UTF-16 rss-proxy.js — blocks merge
P1	2	www normalization + redirect security
P2	3	Dead import, stripped comments, CI
P3	2	Minor suggestions

The UTF-16 encoding on rss-proxy.js is the showstopper — re-save as UTF-8. After that, the www. normalization parity gap should be addressed to avoid silent dev/prod differences. Everything else is non-blocking.

…d redirect security - Re-save api/rss-proxy.js as UTF-8 without BOM (was UTF-16 LE, broke Vercel edge runtime) - Add www. hostname normalization to dev RSS proxy (parity with production) - Switch dev proxy from redirect:'follow' to redirect:'manual' with explicit redirect domain validation against the allowlist (parity with production)

facusturla · 2026-02-27T20:16:18Z

Addresses second review (P0 + P1)

P0 — api/rss-proxy.js UTF-16 encoding fixed. The file had an FF FE BOM
and null bytes between characters (Windows editor saving as "Unicode"). Re-saved
as UTF-8 without BOM. This was likely causing the Vercel CI deploy failures.
P1a — www. hostname normalization added to dev proxy. The dev rssProxyPlugin
in vite/plugins.ts now normalizes hostnames the same way production does:
checking the raw hostname, the bare (no-www.) form, and the www.-prefixed
form. Feeds accessed without www. no longer 403 in dev.
P1b — Dev proxy redirect security hardened. Changed from redirect: 'follow'
(which silently follows to any domain) to redirect: 'manual' with explicit
redirect domain validation against the allowlist — matching production behavior.
Follows up to 5 redirects, validating each target hostname.
P2 — VitePWA import is NOT dead code. It's consumed at line 34 of
vite.config.ts in the VitePWA({ ... }) call. The plugins.ts file exports
individual dev-server middleware plugins; VitePWA is a build plugin configured
directly in the main config.
P2 — All three explanatory comments confirmed present in vite.config.ts
(Geospatial bundles, onnxruntime eval, locale chunk prefix).

koala73 · 2026-02-28T05:42:44Z

Merged under #499 Thank you

facusturla · 2026-03-02T14:37:02Z

Thank you for letting me participate in this amaizing proyect!

koala73 assigned facusturla Feb 26, 2026

facusturla force-pushed the refactor/modular-vite-windows-fix branch 2 times, most recently from 545bd31 to 66b8095 Compare February 26, 2026 13:31

koala73 reviewed Feb 26, 2026

View reviewed changes

facusturla force-pushed the refactor/modular-vite-windows-fix branch from 5ae50dd to 635722f Compare February 26, 2026 19:10

koala73 force-pushed the main branch from cc2088a to 74de5f3 Compare February 27, 2026 14:14

facusturla force-pushed the refactor/modular-vite-windows-fix branch from 635722f to 37e0b01 Compare February 27, 2026 14:57

facusturla added 2 commits February 27, 2026 15:52

refactor: address PR review — fix __dirname, remove process leak, nor…

24c3d3f

…malize indentation, type activeMeta, use cross-env-shell

facusturla force-pushed the refactor/modular-vite-windows-fix branch from 866b0e2 to 3cc1513 Compare February 27, 2026 18:54

koala73 reviewed Feb 27, 2026

View reviewed changes

koala73 mentioned this pull request Feb 28, 2026

fix: use cross-env for Windows-compatible npm scripts #499

Merged

4 tasks

koala73 closed this Feb 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: modularize vite.config.ts and fix Windows script compatibility#379

refactor: modularize vite.config.ts and fix Windows script compatibility#379
facusturla wants to merge 3 commits into
koala73:mainfrom
facusturla:refactor/modular-vite-windows-fix

facusturla commented Feb 25, 2026

Uh oh!

vercel Bot commented Feb 25, 2026

Uh oh!

koala73 commented Feb 26, 2026

Uh oh!

facusturla commented Feb 26, 2026

Uh oh!

koala73 left a comment

Uh oh!

facusturla commented Feb 26, 2026

Uh oh!

facusturla commented Feb 27, 2026

Uh oh!

koala73 left a comment

Uh oh!

facusturla commented Feb 27, 2026

Uh oh!

koala73 commented Feb 28, 2026

Uh oh!

facusturla commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

facusturla commented Feb 25, 2026

Uh oh!

vercel Bot commented Feb 25, 2026

Uh oh!

koala73 commented Feb 26, 2026

Code Review

BLOCKING

SUGGESTIONS

NITPICK

Uh oh!

facusturla commented Feb 26, 2026

Uh oh!

koala73 left a comment

Choose a reason for hiding this comment

PR Review: Modularize vite.config.ts + Windows script compatibility

Overall

BLOCKING

SUGGESTIONS

Summary

Uh oh!

facusturla commented Feb 26, 2026

Addresses PR review feedback

Blocking fixes

Suggestions addressed

Rebase note

Uh oh!

facusturla commented Feb 27, 2026

Updated PR — rebased on latest main

Uh oh!

koala73 left a comment

Choose a reason for hiding this comment

Code Review Summary

Findings

P0 — Critical

P1 — High

P2 — Medium

P3 — Low

What the PR Gets Right

TL;DR

Uh oh!

facusturla commented Feb 27, 2026

Addresses second review (P0 + P1)

Uh oh!

koala73 commented Feb 28, 2026

Uh oh!

facusturla commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Updated PR — rebased on latest `main`