Skip to content
New issue

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

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

Already on GitHub? # to your account

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

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Jan 7, 2025

lforst
lforst previously approved these changes Jan 7, 2025
Copy link
Member

@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

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.

@@ -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

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
Member

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

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
@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
| 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
| 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
| 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!
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Injected global variable _global can clash with user code
2 participants