Skip to content

fix!: Wrap injected code in block-statement to contain scope #646

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 15, 2025
Merged

fix!: Wrap injected code in block-statement to contain scope #646

merged 3 commits into from
Jan 15, 2025

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Jan 7, 2025 •
edited
Loading

lforst
lforst previously approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@lforst lforst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it maybe be enough to just wrap the snippet in {...}. May save some bytes.

@timfish
Copy link
Collaborator Author

timfish commented Jan 7, 2025 •
edited
Loading

Would it maybe be enough to just wrap the snippet in {...}. May save some bytes.

That doesn't work with var because of scoping rules. We would need to use let or const in this case.

timfish
timfish commented Jan 7, 2025
View reviewed changes
packages/bundler-plugin-core/src/utils.ts
@@ -344,7 +346,7 @@ export function generateModuleMetadataInjectorCode(metadata: any) {
// The checks are to support as many environments as possible. (Node.js, Browser, webworkers, etc.)
// We are merging the metadata objects in case modules are bundled twice with the plugin
return `{
var _sentryModuleMetadataGlobal =
const _sentryModuleMetadataGlobal =
Copy link
Collaborator Author

@timfish timfish Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this too because block scoping rules for var mean that it wasn't working as expected

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am all for this but wondering whether we should cut a major in case somebody uses the bundler plugins for apps in an env where const isn't a thing yet.

Copy link
Collaborator Author

@timfish timfish Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind which way we go but we should either:

  1. Use var and wrap in self-invoking function
  2. Use var and wrap in try/catch
  3. Use block-statement with const or let

@timfish timfish requested a review from lforst January 7, 2025 13:18
@timfish timfish dismissed lforst’s stale review January 7, 2025 13:31

made more changes

@timfish timfish changed the title fix: Wrap injected code in self-invoking function to contain scope fix: Wrap injected code in block-statement to contain scope Jan 9, 2025
@timfish timfish mentioned this pull request Jan 9, 2025
Closed
@lforst lforst changed the title fix: Wrap injected code in block-statement to contain scope fix!: Wrap injected code in block-statement to contain scope Jan 15, 2025
@lforst lforst merged commit b734d45 into getsentry:main Jan 15, 2025
18 checks passed
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 15, 2025
@renovate
Update dependency @sentry/vite-plugin to v3
714e9a9 
| datasource | package             | from   | to    |
| ---------- | ------------------- | ------ | ----- |
| npm        | @sentry/vite-plugin | 2.23.0 | 3.0.0 |


## [v3.0.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#300)

##### Breaking Changes

-   Injected code will now use `let`, which was added in ES6 (ES2015).
    This means that ES6 is the minimum JavaScript version that the Sentry bundler plugins support.

-   Deprecated options have been removed:
    -   `deleteFilesAfterUpload` - Use `filesToDeleteAfterUpload` instead
    -   `bundleSizeOptimizations.excludePerformanceMonitoring` - Use `bundleSizeOptimizations.excludeTracing` instead
    -   `_experiments.moduleMetadata` - Use `moduleMetadata` instead
    -   `cleanArtifacts` - Did not do anything

##### List of Changes

-   fix!: Wrap injected code in block-statement to contain scope ([#646](getsentry/sentry-javascript-bundler-plugins#646))
-   chore!: Remove deprecated options ([#654](getsentry/sentry-javascript-bundler-plugins#654))
-   feat(logger): Use console methods respective to log level ([#652](getsentry/sentry-javascript-bundler-plugins#652))
-   fix(webpack): Ensure process exits when done ([#653](getsentry/sentry-javascript-bundler-plugins#653))
-   fix: Use correct replacement matcher for `bundleSizeOptimizations.excludeTracing` ([#644](getsentry/sentry-javascript-bundler-plugins#644))

Work in this release contributed by [@jdelStrother](https://github.com/jdelStrother). Thank you for your contribution!
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 15, 2025
@renovate
Update dependency @sentry/vite-plugin to v3
26d347c 
| datasource | package             | from   | to    |
| ---------- | ------------------- | ------ | ----- |
| npm        | @sentry/vite-plugin | 2.23.0 | 3.0.0 |


## [v3.0.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#300)

##### Breaking Changes

-   Injected code will now use `let`, which was added in ES6 (ES2015).
    This means that ES6 is the minimum JavaScript version that the Sentry bundler plugins support.

-   Deprecated options have been removed:
    -   `deleteFilesAfterUpload` - Use `filesToDeleteAfterUpload` instead
    -   `bundleSizeOptimizations.excludePerformanceMonitoring` - Use `bundleSizeOptimizations.excludeTracing` instead
    -   `_experiments.moduleMetadata` - Use `moduleMetadata` instead
    -   `cleanArtifacts` - Did not do anything

##### List of Changes

-   fix!: Wrap injected code in block-statement to contain scope ([#646](getsentry/sentry-javascript-bundler-plugins#646))
-   chore!: Remove deprecated options ([#654](getsentry/sentry-javascript-bundler-plugins#654))
-   feat(logger): Use console methods respective to log level ([#652](getsentry/sentry-javascript-bundler-plugins#652))
-   fix(webpack): Ensure process exits when done ([#653](getsentry/sentry-javascript-bundler-plugins#653))
-   fix: Use correct replacement matcher for `bundleSizeOptimizations.excludeTracing` ([#644](getsentry/sentry-javascript-bundler-plugins#644))

Work in this release contributed by [@jdelStrother](https://github.com/jdelStrother). Thank you for your contribution!
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Jan 17, 2025
@renovate
Update dependency @sentry/vite-plugin to v3
6f25fdb 
| datasource | package             | from   | to    |
| ---------- | ------------------- | ------ | ----- |
| npm        | @sentry/vite-plugin | 2.23.0 | 3.0.0 |


## [v3.0.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#300)

##### Breaking Changes

-   Injected code will now use `let`, which was added in ES6 (ES2015).
    This means that ES6 is the minimum JavaScript version that the Sentry bundler plugins support.

-   Deprecated options have been removed:
    -   `deleteFilesAfterUpload` - Use `filesToDeleteAfterUpload` instead
    -   `bundleSizeOptimizations.excludePerformanceMonitoring` - Use `bundleSizeOptimizations.excludeTracing` instead
    -   `_experiments.moduleMetadata` - Use `moduleMetadata` instead
    -   `cleanArtifacts` - Did not do anything

##### List of Changes

-   fix!: Wrap injected code in block-statement to contain scope ([#646](getsentry/sentry-javascript-bundler-plugins#646))
-   chore!: Remove deprecated options ([#654](getsentry/sentry-javascript-bundler-plugins#654))
-   feat(logger): Use console methods respective to log level ([#652](getsentry/sentry-javascript-bundler-plugins#652))
-   fix(webpack): Ensure process exits when done ([#653](getsentry/sentry-javascript-bundler-plugins#653))
-   fix: Use correct replacement matcher for `bundleSizeOptimizations.excludeTracing` ([#644](getsentry/sentry-javascript-bundler-plugins#644))

Work in this release contributed by [@jdelStrother](https://github.com/jdelStrother). Thank you for your contribution!
@Lms24 Lms24 mentioned this pull request Jan 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lforst lforst lforst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Injected global variable _global can clash with user code
2 participants
@timfish @lforst