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

feat(core): Expose bundler plugin primitives via createSentryBuildPluginManager #713

Merged

Conversation

lforst
Copy link
Member

@lforst lforst commented Apr 1, 2025

This PR adds a createSentryBuildPluginManager function to the @sentry/bundler-plugin-core package that exposes most of the primitives that are necessary to build a Sentry bundler plugin, or a Sentry plugin for JS build tooling.

  • Why do we need a "manager" instance for this and not just expose the primitives directly?
    • A lot of the logic in the bundler plugins comes from the options that are passed in. If we were to expose the primitives directly, the option normalization would always have to be handled by the consuming package and that would get very cumbersome and prone to errors and inconsistencies.

This PR leaves the core package in an extremely desolate state. Thisis because it required major refactorings but I didn't want to clean stuff up in the same PR because it would get awful to review.

@lforst lforst requested a review from Copilot April 1, 2025 15:11
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR exposes new bundler plugin primitives by refactoring several plugins (Webpack, Vite, Rollup, Esbuild, and Bundler Plugin Core) to simplify dependency management and unify error handling through a centralized Sentry build plugin manager. Key changes include:

  • Injecting and using a new createDependencyOnBuildArtifacts function across various plugins
  • Refactoring telemetry, release management, and sourcemap deletion logic to delegate functionality to the centralized plugin manager
  • Simplifying the debug ID upload workflow by delegating the upload and error handling to the plugin manager

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/webpack-plugin/src/index.ts Added new dependency injection parameter and cleanup for upload logic
packages/vite-plugin/src/index.ts Updated plugin interface to include logger and dependency cleanup
packages/rollup-plugin/src/index.ts Adjusted parameter ordering and delegated dependency cleanup
packages/esbuild-plugin/src/index.ts Introduced dependency cleanup in the esbuild debug ID upload plugin
packages/bundler-plugin-core/src/* Refactored telemetry, sourcemap deletion, release management, and debug ID upload to use the plugin manager
packages/integration-tests/fixtures/telemetry/telemetry.test.ts, packages/vite-plugin/test/public-api.test.ts, packages/rollup-plugin/test/public-api.test.ts Removed tests related to legacy behavior in development mode
Comments suppressed due to low confidence (5)

packages/webpack-plugin/src/index.ts:127

  • [nitpick] Consider renaming 'createDependencyOnBuildArtifacts' to a name that more clearly indicates it returns a cleanup function for build artifact dependencies, e.g. 'registerBuildArtifactDependency'.
createDependencyOnBuildArtifacts: () => () => void,

packages/vite-plugin/src/index.ts:48

  • [nitpick] Ensure the parameter order and naming in 'viteDebugIdUploadPlugin' mirrors the patterns used in the webpack and rollup plugins for consistency.
upload: (buildArtifacts: string[]) => Promise<void>,

packages/bundler-plugin-core/src/debug-id-upload.ts:21

  • Verify that delegating the entire sourcemap upload process to 'uploadSourcemaps' does not omit any necessary error handling or cleanup previously performed within this function.
await sentryBuildPluginManager.uploadSourcemaps(buildArtifactPaths);

packages/bundler-plugin-core/src/plugins/sourcemap-deletion.ts:14

  • Double-check that removing the explicit error handling logic in the file deletion plugin in favor of 'deleteArtifacts' still provides sufficient feedback in case deletion fails.
await sentryBuildPluginManager.deleteArtifacts();

packages/bundler-plugin-core/src/index.ts:132

  • Confirm that replacing the previous metadata handling with 'sentryBuildPluginManager.bundleMetadata' correctly aggregates and injects all necessary metadata across multiple bundling passes.
const injectionCode = generateModuleMetadataInjectorCode(sentryBuildPluginManager.bundleMetadata);

@lforst lforst changed the title feat(core): Expose bundler plugin primitives feat(core): Expose bundler plugin primitives via createSentryBuildPluginManager Apr 2, 2025
@lforst lforst changed the base branch from main to lforst-primitives-feature-branch April 2, 2025 07:48
@lforst lforst marked this pull request as ready for review April 2, 2025 07:48
@AbhiPrasad
Copy link
Member

What's the motivation? To eventually move off of unplugin?

@lforst
Copy link
Member Author

lforst commented Apr 2, 2025

What's the motivation? To eventually move off of unplugin?

It's basically needed to support Turbopack in Next.js. See getsentry/sentry-javascript#15779.

Turbopack doesn't have a plugin system so we need to access the primitives from the Next.js layer for now.

@lforst lforst merged commit d5ada3c into lforst-primitives-feature-branch Apr 2, 2025
18 checks passed
@lforst lforst deleted the lforst-bundler-plugin-primitives branch April 2, 2025 08:04
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.

2 participants