refactor: auto-enable image proxy handlers from registry config#629
refactor: auto-enable image proxy handlers from registry config#629
Conversation
Remove separate `googleStaticMapsProxy` module option — the handler is now registered automatically when `registry.googleMaps` is set. The API key and cacheMaxAge are injected into runtime config from the registry entry, removing the need for explicit opt-in.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
📝 WalkthroughWalkthroughThe pull request refactors the Google Static Maps proxy configuration from an explicit module option to a registry-driven approach. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes DetailsThe changes involve an architectural shift from explicit configuration to registry-driven activation across three files. The logic modifications are straightforward but require verification that the new presence-based checks correctly replace the previous enabled-flag pattern and that removing the server-side guard maintains expected behavior. The heterogeneous nature of changes—module configuration restructuring, component conditional logic simplification, and guard removal—demands separate reasoning for each area, placing this in moderate complexity territory. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue (2)
459-459:⚠️ Potential issue | 🟠 MajorInconsistent condition: API key leaks to client when proxy is enabled.
Line 459 checks
proxyConfig?.enabledto decide whether to omit the API key from the placeholder URL. Sinceenabledno longer exists in the config, this always evaluates toundefined(falsy), soapiKeyis always included in the query string—even when using the proxy where the key should be injected server-side.This leaks the API key to the client unnecessarily when the proxy is active.
Update to check for the presence of
proxyConfig:Proposed fix
// Only include API key if not using proxy (proxy injects it server-side) - key: proxyConfig?.enabled ? undefined : apiKey, + key: proxyConfig ? undefined : apiKey,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue` at line 459, The condition that determines whether to include the API key is using proxyConfig?.enabled which no longer exists and causes apiKey to always be exposed; update the check in the object where key: proxyConfig?.enabled ? undefined : apiKey to instead test for the presence of proxyConfig (e.g., key: proxyConfig ? undefined : apiKey) so when a proxy configuration object exists the client-side URL omits apiKey; locate this change in ScriptGoogleMaps.vue around the object that sets key and replace the condition accordingly.
419-428:⚠️ Potential issue | 🟡 MinorInconsistent condition:
proxyConfig?.enabledno longer exists.Line 419 still checks
!proxyConfig?.enabled, but the new config structure (per context snippet frommodule.ts) only contains{ cacheMaxAge: 3600 }with noenabledfield. This condition will always evaluate totrue(sinceundefinedis falsy), causing the preconnect/dns-prefetch link to be added even when the proxy is enabled—which is unnecessary and potentially misleading.Update to check for the presence of
proxyConfiginstead:Proposed fix
-if (import.meta.server && !proxyConfig?.enabled) { +if (import.meta.server && !proxyConfig) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue` around lines 419 - 428, The condition currently uses proxyConfig?.enabled which no longer exists; update the server-side check in ScriptGoogleMaps.vue so the link is only added when no proxyConfig is provided (e.g., keep the import.meta.server check but replace the !proxyConfig?.enabled test with a presence check like !proxyConfig or proxyConfig == null) so useHead is called only when the proxy is absent; edit the block around useHead (the import.meta.server guard and its link entry) to reference proxyConfig presence instead of proxyConfig?.enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/runtime/components/GoogleMaps/ScriptGoogleMaps.vue`:
- Line 459: The condition that determines whether to include the API key is
using proxyConfig?.enabled which no longer exists and causes apiKey to always be
exposed; update the check in the object where key: proxyConfig?.enabled ?
undefined : apiKey to instead test for the presence of proxyConfig (e.g., key:
proxyConfig ? undefined : apiKey) so when a proxy configuration object exists
the client-side URL omits apiKey; locate this change in ScriptGoogleMaps.vue
around the object that sets key and replace the condition accordingly.
- Around line 419-428: The condition currently uses proxyConfig?.enabled which
no longer exists; update the server-side check in ScriptGoogleMaps.vue so the
link is only added when no proxyConfig is provided (e.g., keep the
import.meta.server check but replace the !proxyConfig?.enabled test with a
presence check like !proxyConfig or proxyConfig == null) so useHead is called
only when the proxy is absent; edit the block around useHead (the
import.meta.server guard and its link entry) to reference proxyConfig presence
instead of proxyConfig?.enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39676e23-e4d5-4ecc-8346-8fa3797c62e4
📒 Files selected for processing (3)
src/module.tssrc/runtime/components/GoogleMaps/ScriptGoogleMaps.vuesrc/runtime/server/google-static-maps-proxy.ts
💤 Files with no reviewable changes (1)
- src/runtime/server/google-static-maps-proxy.ts
1 participant
❓ Type of change
📚 Description
Removes the separate
googleStaticMapsProxymodule option — the Google Static Maps proxy handler is now auto-registered whenregistry.googleMapsis configured. The API key andcacheMaxAgeare injected into runtime config automatically.Before:
After:
scripts.googleStaticMapsProxyconfig option is removed. The proxy now auto-enables whenregistry.googleMapsis set.Verification