feat(browser): Add environment variable support for Spotlight configuration #18198
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
size-limit report 📦
|
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
…ration - Add support for multiple framework/bundler-specific environment variables with proper precedence - SENTRY_SPOTLIGHT (highest priority - base name, supported natively by many bundlers) - PUBLIC_SENTRY_SPOTLIGHT (SvelteKit, Astro, Qwik) - NEXT_PUBLIC_SENTRY_SPOTLIGHT (Next.js) - VITE_SENTRY_SPOTLIGHT (Vite) - NUXT_PUBLIC_SENTRY_SPOTLIGHT (Nuxt.js) - REACT_APP_SENTRY_SPOTLIGHT (Create React App) - VUE_APP_SENTRY_SPOTLIGHT (Vue CLI) - GATSBY_SENTRY_SPOTLIGHT (Gatsby) - Add defensive environment variable access via process.env (transformed by all major bundlers) - Move envToBool utility from node-core to core for shared usage - Add resolveSpotlightOptions utility for consistent precedence rules - Update node-core and aws-serverless to use shared utilities - Add comprehensive tests for all new utilities and SDK integration Note: import.meta.env is intentionally not checked because bundlers only replace static references (e.g., import.meta.env.VITE_VAR) at build time, not dynamic access. All major bundlers transform process.env references, making it the universal solution.
BYK
force-pushed
the
feat/spotlight-environment-variable-support
branch
from
92ffd33 to
6cb5513
Compare
BYK
force-pushed
the
feat/spotlight-environment-variable-support
branch
from
4f6dcd0 to
fda3d61
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [x] If you've added code that should be tested, please add tests. - [x] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). Fixes a test failure where `getSpotlightConfig` was returning empty or whitespace-only strings for `SENTRY_SPOTLIGHT` environment variables, instead of `undefined`. This change explicitly filters out such values, aligning the function's behavior with test expectations and preventing invalid Spotlight configurations. --- <a href="https://cursor.com/background-agent?bcId=bc-88bd0365-3796-47b2-bb84-29c93d0430e8"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"></picture></a> <a href="https://cursor.com/agents?id=bc-88bd0365-3796-47b2-bb84-29c93d0430e8"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web" src="https://cursor.com/open-in-web.svg"></picture></a> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Lms24
reviewed
Nov 17, 2025
Member
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
First pass review. I still want to figure out why size bot reports a slight increase for the CDN bundles.
packages/browser/src/utils/env.ts
Outdated
Comment on lines
3
to
9
| * Checks process.env which is transformed by most bundlers (Webpack, Vite, Rollup, Rspack, Parcel, etc.) | ||
| * at build time. | ||
| * | ||
| * Note: We don't check import.meta.env because: | ||
| * 1. Bundlers only replace static references like `import.meta.env.VITE_VAR`, not dynamic access | ||
| * 2. Dynamic access causes syntax errors in unsupported environments | ||
| * 3. Most bundlers transform process.env references anyway |
Member
There was a problem hiding this comment.
m: My impression while doing some research for #18050 (comment) was what very few bundlers inject process.env into browser bundles. For example, this code will not work in Angular.
Vite instructs checking on import.meta.env, so not sure if it double-writes to process.env (my gut feeling is no).
Did you test this in the respective frameworks? Maybe I'm also misinformed and this works just fine 😅 Ideally we can add an e2e test for at least NextJS and some Vite-based framework app to be sure.
Comment on lines
62
to
64
|
|
||
| // No Spotlight configuration found in environment | ||
| return undefined; |
Member
There was a problem hiding this comment.
l: neat size trick: You can let the function return void and simply omit the return undefined here. Saves a couple of bytes and JS returns undefined anyway. 😅 (but feel free to ignore since this is shaken out for prod builds anyway)
Before submitting a pull request, please take a look at our [Contributing](https://github.com/getsentry/sentry-javascript/blob/master/CONTRIBUTING.md) guidelines and verify: - [x] If you've added code that should be tested, please add tests. - [x] Ensure your code lints and the test suite passes (`yarn lint`) & (`yarn test`). --- This PR addresses critical feedback regarding environment variable detection, particularly for Vite-based frameworks. **Key Changes:** * **`packages/browser/src/utils/env.ts`**: The `getEnvValue` function now checks both `process.env` (for Webpack, Next.js, CRA) and `import.meta.env` (for Vite, Astro, SvelteKit). This ensures that environment variables (like those for Spotlight) are correctly detected across a wider range of bundlers and frameworks, fixing a significant compatibility issue. * **`packages/browser/test/utils/env.test.ts`**: Updated unit tests to focus on `process.env` scenarios. Added a note explaining that `import.meta.env` cannot be unit tested due to its read-only, compile-time nature and is covered by e2e tests. * **`packages/browser/src/utils/spotlightConfig.ts`**: Added a comment to clarify the explicit `return undefined` for readability, noting its optimization in production builds. --- <a href="https://cursor.com/background-agent?bcId=bc-d6001dc9-59fc-42eb-8354-a573e3ae71a9"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-cursor-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in Cursor" src="https://cursor.com/open-in-cursor.svg"></picture></a> <a href="https://cursor.com/agents?id=bc-d6001dc9-59fc-42eb-8354-a573e3ae71a9"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/open-in-web-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web" src="https://cursor.com/open-in-web.svg"></picture></a> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
| if (typeof import.meta !== 'undefined' && import.meta.env) { | ||
| // @ts-expect-error import.meta.env is typed differently in different environments | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| const value = import.meta.env[key]; |
There was a problem hiding this comment.
Bug: Dynamic Env Access Breaks Bundler Builds
Dynamic property access import.meta.env[key] won't work with build-time environment variable replacement in bundlers like Vite, Astro, and SvelteKit. These bundlers require static property access like import.meta.env.VARIABLE_NAME for compile-time replacement. The dynamic bracket notation will always return undefined in production builds because the object doesn't exist at runtime.
Collaborator
There was a problem hiding this comment.
@BYK I have no idea if this is true but seems relevant! Sounds like in these cases the bundler only replaces the full string/identifier?
timfish
reviewed
Nov 18, 2025
Collaborator
timfish
left a comment
timfish
left a comment
There was a problem hiding this comment.
Looks good but needs some e2e tests of some kind otherwise we could inadvertently break this in the future.
It should be quite easy to test Vite by adding some additional tests to dev-packages/bundler-tests with the specific variables set and then check they make it into the bundle.
Then as @Lms24 says, a Nextjs test would probably be a good idea too!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
![https://cursor.com/open-in-cursor-dark.svg"><source](https://cursor.com/open-in-cursor-dark.svg"><source){kind=link}
![https://cursor.com/open-in-cursor-light.svg"><img](https://cursor.com/open-in-cursor-light.svg"><img){kind=link}
![https://cursor.com/open-in-cursor.svg"></picture></a> <a](https://cursor.com/open-in-cursor.svg"></picture></a> <a){kind=link}
![https://cursor.com/open-in-web-dark.svg"><source](https://cursor.com/open-in-web-dark.svg"><source){kind=link}
![https://cursor.com/open-in-web-light.svg"><img](https://cursor.com/open-in-web-light.svg"><img){kind=link}
![https://cursor.com/open-in-web.svg"></picture></a](https://cursor.com/open-in-web.svg"></picture></a){kind=link}
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements full Spotlight spec with support for multiple framework-specific
environment variable prefixes. Adds defensive environment variable access
for both process.env and import.meta.env to support various bundlers.
Supported environment variables (in priority order):
PUBLIC_SENTRY_SPOTLIGHT(SvelteKit, Astro, Qwik)NEXT_PUBLIC_SENTRY_SPOTLIGHT(Next.js)VITE_SENTRY_SPOTLIGHT(Vite)NUXT_PUBLIC_SENTRY_SPOTLIGHT(Nuxt)REACT_APP_SENTRY_SPOTLIGHT(Create React App)VUE_APP_SENTRY_SPOTLIGHT(Vue CLI)GATSBY_SENTRY_SPOTLIGHT(Gatsby)SENTRY_SPOTLIGHT(base/official)SENTRY_SPOTLIGHTis last as in environments like Docker Compose, we actually make the front-end env variable different than the baseSENTRY_SPOTLIGHTone -- the backends need to reachdocker.host.internalwhereas front-ends always needlocalhostas we assume the browser runs on the same host with Spotlight.Refactors envToBool utility from node-core to core package for shared usage.
Adds resolveSpotlightOptions utility to ensure consistent precedence rules
across Browser and Node SDKs.
Includes comprehensive test coverage for all new utilities and integration
tests for environment variable precedence behavior.