Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser lib contains variable "chrome" that conflicts with the global "chrome" variable #6880

Closed
3 tasks done
wesleyyee opened this issue Jan 19, 2023 · 10 comments · Fixed by #10874
Closed
3 tasks done
Labels
Package: browser Issues related to the Sentry Browser SDK Type: Bug

Comments

@wesleyyee
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/browser

SDK Version

7.31.1

Framework Version

No response

Link to Sentry event

No response

SDK Setup

import * as Sentry from '@sentry/browser';
import { BrowserTracing } from '@sentry/tracing';

Sentry.init({
dsn: SENTRY_DSN,
integrations: [new BrowserTracing()],

// Set tracesSampleRate to 1.0 to capture 100%
// of transactions for performance monitoring.
// We recommend adjusting this value in production
tracesSampleRate: isProduction ? 0.5 : 1.0,
});

Steps to Reproduce

  • Import sentry into script
  • Load script in browser

Expected Result

No conflicting variable

Actual Result

Uncaught SyntaxError: Identifier 'chrome' has already been declared

@wesleyyee wesleyyee changed the title Browser lib contains variable chrome that conflicts with the global chrome variable Browser lib contains variable "chrome" that conflicts with the global "chrome" variable Jan 19, 2023
@AbhiPrasad
Copy link
Member

Hey @wesleyyee, thanks for writing in. I tried to reproduce this, but was unable to: https://codesandbox.io/s/global-chrome-variable-config-jpu1bm?file=/src/index.js. Could you give some details about how you are bundling you application, and what your bundler config looks like?

It would be great if you could provide a reproduction of this issue to help us debug further.

@AbhiPrasad AbhiPrasad added Status: Needs Reproduction Package: browser Issues related to the Sentry Browser SDK labels Jan 19, 2023
@wesleyyee
Copy link
Author

Thanks @AbhiPrasad we are using rollup to bundle and the config looks like


export default {
  input: 'barcart/index.js',
  output: {
    file: 'barcart/dist/barcart.js',
    format: 'es',
  },
  plugins: [nodeResolve({ browser: true })],
};

@wesleyyee
Copy link
Author

wesleyyee commented Jan 19, 2023

this seems to be the variable that collides with the global chrome object
https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/stack-parsers.ts#L37

@AbhiPrasad
Copy link
Member

The chrome declaration is not being exported, so I don't know how it ends up in your final bundle since the bundler should be isolating it to it's own lexical scope. I think there is something weird happening with your final bundle that gets generated, maybe it's something to do with nodeResolve?

Maybe you have to dedupe your sentry imports if there are duplicates appearing?

From looking around I can only see SAP/ui5-webcomponents#4978 which hit a similar problem (also with rollup).

Do you know what else in your app could be declaring a global chrome object? If you can probably find this by searching through your final generated bundle.

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@AbhiPrasad AbhiPrasad closed this as not planned Won't fix, can't repro, duplicate, stale Feb 10, 2023
@dmajkic
Copy link

dmajkic commented Feb 29, 2024

Could you please review link from @wesleyyee where chrome: is defined. We also had issue with Vite option that disables identifier minification that raised "chrome" conflict with global chrome variable

@mydea
Copy link
Member

mydea commented Feb 29, 2024

This link: https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/stack-parsers.ts#L37 does not lead to anything chrome related anymore. If you're still seeing an issue, could you open a new issue with as much info as possible, so we can take a look at reproducing/fixing the issue? thank you!

@dmajkic
Copy link

dmajkic commented Feb 29, 2024

Line 61:
https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/stack-parsers.ts#L61

This was from logs: processed js received on browser side, where we got error:
image

@AbhiPrasad
Copy link
Member

This is breaking my brain a little bit, but to unblock I opened #10874 - but I think that is not the long term solution.

We also had issue with Vite option that disables identifier minification that raised "chrome" conflict with global chrome variable

@dmajkic mind elaborating why you're doing this? It does increase bundle size substantially.

AbhiPrasad added a commit that referenced this issue Mar 4, 2024
resolves #6880

Using `const chrome = ...` or similar breaks certain setups based on
their bundler config. This makes changes to browser code so that we
never declare a variable named `chrome`.

I tried setting up https://eslint.org/docs/latest/rules/id-denylist to
enforce this, but this unfortunately also looks as types declarations
(so `type K = { chrome: ... }` is problematic.
AbhiPrasad added a commit that referenced this issue Mar 7, 2024
resolves #6880

Using `const chrome = ...` or similar breaks certain setups based on
their bundler config. This makes changes to browser code so that we
never declare a variable named `chrome`.

I tried setting up https://eslint.org/docs/latest/rules/id-denylist to
enforce this, but this unfortunately also looks as types declarations
(so `type K = { chrome: ... }` is problematic.
@AbhiPrasad
Copy link
Member

Released https://github.com/getsentry/sentry-javascript/releases/tag/7.106.0 which should address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: browser Issues related to the Sentry Browser SDK Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants