Skip to content

fix(bundler-plugins): integration with monorepo#21342

Open
isaacs wants to merge 739 commits into
developfrom
feat/merge-in-sentry-javascript-bundler-plugins
Open

fix(bundler-plugins): integration with monorepo#21342
isaacs wants to merge 739 commits into
developfrom
feat/merge-in-sentry-javascript-bundler-plugins

Conversation

@isaacs

@isaacs isaacs commented Jun 4, 2026

Copy link
Copy Markdown
Member

Note: the commit log in this PR is ridiculous, because it intentionally preserves the history of the sentry-javascript-bundler-plugins repo. Probably best to review locally. Attempting to do a rebase or squash merge will probably break GitHub and maybe the world 😅

Integrate the newly merged in @sentry/bundler-plugins project to the
sentry-javascript monorepo.

  • Integration tests are updated to use the named submodule exports on
    the @sentry/bundler-plugins package, rather than the per-platform
    re-export shims.
  • Lint and naming conventions brought in line with project standards.
  • Bundler-plugins integration tests now run on CI.

The version has not been updated from its independent 5.x line, though
it may be a good idea to bump it up to align with the 10.x line that the
rest of the monorepo uses.

getsentry-bot and others added 30 commits January 10, 2025 12:42
Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
…ludeTracing` (#644)

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
Revert the default behaviour of the webpack plugin to no longer exit the process. Instead, users can set an experimental flag to force exiting the process.
…terUpload` (#677)

Widen the accepted type for `filesToDeleteAfterSourcemaps` to allow us (as well as users) to pass in a `Promise<string | string[]>` to do so. This promise can resolve whenever we know what to set and we await the promise before calling `glob` to get all file paths to delete.
Update changelog for 3.2.0
@chargome chargome self-requested a review June 5, 2026 11:56
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from 8046230 to 4008b69 Compare June 5, 2026 14:01
Comment thread dev-packages/bundler-plugin-integration-tests/fixtures/esbuild/package.json Outdated
Comment thread packages/bundler-plugins/sentry-esbuild-debugid-injection-file.js
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from 4008b69 to 1fe3d0c Compare June 5, 2026 14:25
Comment thread packages/bundler-plugins/src/core/options-mapping.ts
Comment thread packages/bundler-plugins/src/core/build-plugin-manager.ts
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from 1fe3d0c to 6882d3e Compare June 5, 2026 15:05
Comment thread package.json Outdated
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch 2 times, most recently from 2420de2 to a37151b Compare June 5, 2026 15:31
Comment thread packages/bundler-plugins/src/webpack/webpack4and5.ts
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from a37151b to 2d30000 Compare June 5, 2026 15:54
Comment thread packages/bundler-plugins/src/babel-plugin/index.ts
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch 2 times, most recently from 239808f to baf5845 Compare June 7, 2026 18:11
@timfish

timfish commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

For the new package, this list needs to be considered:
https://github.com/getsentry/sentry-javascript/blob/develop/docs/new-sdk-release-checklist.md

I think the new package is missing README.md and LICENCE. We can fix here or complete this checklist in another PR?

@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from baf5845 to cba358e Compare June 8, 2026 16:15
Comment thread packages/bundler-plugins/src/core/build-plugin-manager.ts
@isaacs

isaacs commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

@timfish I copied over the LICENSE and README.md from the bundler-plugins-core package, since that's about the closest analogue to this refactor. Because it's semi-internal, I think some of the other "new SDK" steps don't really apply, but we can address them either before or after landing this, I don't really have a strong opinion about it.

@timfish timfish self-requested a review June 8, 2026 17:29
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from b2c8cef to 3ab9f64 Compare June 11, 2026 14:01
@isaacs isaacs force-pushed the feat/merge-in-sentry-javascript-bundler-plugins branch from 3ab9f64 to 413068a Compare June 11, 2026 16:34
"@sentry/bundler-plugins": "file:../../../bundler-plugins/sentry-bundler-plugins-5.3.0.tgz",
"@sentry/bundler-plugin-core": "file:../../../bundler-plugin-core/sentry-bundler-plugin-core-5.3.0.tgz",
"@sentry/esbuild-plugin": "file:../../../esbuild-plugin/sentry-esbuild-plugin-5.3.0.tgz",
"@sentry/babel-plugin-component-annotate": "file:../../../babel-plugin-component-annotate/sentry-babel-plugin-component-annotate-5.3.0.tgz"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

pnpm override paths one level too deep

High Severity

Fixture pnpm.overrides use file:../../../… for packed plugin tarballs, but from fixtures/<bundler>/ three parent segments resolve to dev-packages/, not the integration-tests package root. setup.mjs runs pnpm install in those fixture dirs, so the paths miss the intended tarball locations and installs fail unless unrelated directories exist under dev-packages/.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 413068a. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2449737. Configure here.

"@sentry/bundler-plugins": "file:../../../bundler-plugins/sentry-bundler-plugins-5.3.0.tgz",
"@sentry/bundler-plugin-core": "file:../../../bundler-plugin-core/sentry-bundler-plugin-core-5.3.0.tgz",
"@sentry/esbuild-plugin": "file:../../../esbuild-plugin/sentry-esbuild-plugin-5.3.0.tgz",
"@sentry/babel-plugin-component-annotate": "file:../../../babel-plugin-component-annotate/sentry-babel-plugin-component-annotate-5.3.0.tgz"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wrong fixture tgz override paths

High Severity

Fixture pnpm.overrides still point at standalone-repo tarball paths under dev-packages (bundler-plugins, bundler-plugin-core, esbuild-plugin, etc.). After monorepo integration the packed plugin lives under packages/bundler-plugins, and separate core/esbuild-plugin directories are not present at those relative paths, so pnpm install in fixtures cannot resolve the intended local builds.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2449737. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This isn't quite right, but it does point out a pretty significant oversight. The bundler-plugin integration tests still use the older module names, which are not ported over in this merge.

Those need to be updated to use @sentry/bundler-plugins/<platform> instead of @sentry/<platform>-plugin, and built properly. I'll add another commit to this PR to do that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm, running the full bundler-plugins integration suite in CI is a bit more of a refactor. I think it's best for this PR to leave it in its current semi-broken state, get the code pulled in, and then fix it up to run properly and be integrated into CI. Attempting to do the full nx convention alignment right now would be pretty risky, I think, and while it's not in a great state, it's mostly harmless (doesn't break any other parts of the build, just isn't being tested fully).

@chargome @timfish What do you two think about this approach?

This also excludes several fixtures from the lint/formatting process,
since they are in some cases intentionally broken.
_metaOptions: {
telemetry: {
metaFramework: userOptions._metaOptions?.telemetry?.metaFramework,
bundlerMajorVersion: userOptions._metaOptions?.telemetry?.bundlerMajorVersion,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: A type mismatch exists where bundlerMajorVersion is set on the normalized options at runtime but is missing from its NormalizedOptions type definition.
Severity: LOW

Suggested Fix

Remove the assignment of bundlerMajorVersion from the normalizeUserOptions function in options-mapping.ts since this property is unused and is not part of the NormalizedOptions type. This will resolve the type inconsistency.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/bundler-plugins/src/core/options-mapping.ts#L125

Potential issue: In `normalizeUserOptions`, the `bundlerMajorVersion` property is
assigned to `_metaOptions.telemetry`. However, the type definition for the normalized
options, `NormalizedOptions`, does not include `bundlerMajorVersion` under
`_metaOptions.telemetry`. While this creates a type mismatch where a property exists at
runtime but not in its type declaration, it has no functional impact. The property is
never read from the normalized options object. The value used for telemetry is passed as
a direct parameter to the `setTelemetryDataOnScope` function from the specific bundler
plugin, not from the options object.

Also affects:

  • packages/bundler-plugins/src/core/types.ts:76~80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.