fix(bundler-plugins): Preserve directive prologues during bundle injection

[Andarist]

Bundler plugins inject Sentry code into generated bundles. In some outputs the injection could be placed before directive prologues such as "use strict", causing JavaScript to stop recognizing them as directives.

I found this issue to affect some production cases - I don't have access to their sources so I can't fully say how the original code was authored and what exactly made it lose the strict mode, but the generated output looked like this:

try{!function(){var e="undefined"!=typeof window?window:"undefined"!=typeof global?global:"undefined"!=typeof globalThis?
  globalThis:"undefined"!=typeof self?self:{},t=(new e.Error).stack;t&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[t]="612bd636-5fcc-473a-bdd0-20460245872c",e._sentryDebugIdIdentifier="sentry-dbid-612bd636-5fcc-473a-bdd0-
  20460245872c")}()}catch(e){}"use strict";(self.webpackChunk_N_E=self.webpackChunk_N_E||[]).push([[120],{67232:function(e,t,n){var r,l=n(41498),a=n(90413),o={usingClientEntryPoint:!1,Events:null,Dispatcher:{current:null}};function i(e){var
  t="https://react.dev/errors/"+e;if(1<arguments.length){t+="?args[]="+encodeURIComponent(arguments[1]);

Given the sentry code was injected before the strict mode directive, that changed the meaning of arguments[1] at this position in the app code:

function aW(e, t) {
  if (null !== (t = null !== (t = t.updateQueue) ? t.lastEffect : null)) {
    var n = (t = t.next);
    do {
      if ((n.tag & e) === e) {
        var r = n.create,
          l = n.inst;
        /* arguments[1] no longer refer to the original argument */
        r = r();

        l.destroy = r;
      }
      n = n.next;
    } while (n !== t);
  }
}

That's because in the sloppy mode the assignment to t before the arguments[1] reference changes the arguments content too 🫠 . You can test it out using this isolated sample:

function test(foo) {
  foo = 2;
  console.log(arguments[0]); // 2
}

test(1);

function testStrict(foo) {
  "use strict";
  foo = 2;
  console.log(arguments[0]); // 1
}

testStrict(1);

This PR:

• adds a bunch of tests for edge cases and for source mapping behavior (the latter was already working OK but didn't quite have the coverage)
• replaces simple regex with a more spec-compliant tiny scanner so the proper injection point can be found
• in Webpack BannePlugin can only prepend/append text, as far as I know, it can't just inject into an arbitrary position. So it was replaced with a compilation hook and ReplaceSource plugin. That allows for a fine-grained control at the asset level
• in the case of Rollup, this PR only slightly changes the insertion point calculation - but it doesn't replace the overall mechanism/hooks used
• esbuild has not required any fixes because inject API handles this for us

---

AI disclosure: I have steered it a bunch myself and I understand each line of code added. I ensured (using my own judgement) that all of this matches the project's style and goal but ofc I have much less context on that than the maintainers here. That said, I can address any PR feedback thrown my way.

[timfish]

Seems sensible to me!

[Andarist]

@timfish could u re-approve the CI run? I fixed the test failures (hopefully the last ones). I'd test it out properly locally... but I'm facing a stupid issue of this project having such a big dep graph that I'm currently running out of disk space 🫠 I need to clean it up but didn't have time for that right now. FWIW, I was totally testing this... but through proxy isolated pnpm-based projects. So I, from the start, tested he behavioral changes of this PR - but then I faced some outdated snapshots here 😢

[Andarist]

@timfish I believe I fixed the remaining issues in the test harness. I also tested it through your github actions on my fork (see this run). Some e2e tests are failing but that's because I don't have the required secrets on my fork.

Could you now retrigger the CI here?

[Andarist]

I had to sync this with develop given the merge conflicts, this this will require now a CI re-run.

[cursor]

Stale Bugbot comment from a previous run.

[timfish]

Thanks, will review this week!

[timfish]

Thanks, the scanner and the move to processAssets both look right to me. A few follow-ups inline, plus one that has no diff line to hang on: COMMENT_USE_STRICT_REGEX in packages/bundler-plugins/src/core/index.ts has no users left after this change. Can you delete it (and its CodeQL note)?

[timfish]

We already have a scanner for this in packages/nextjs/src/config/loaders/valueInjectionLoader.ts (findInjectionIndexAfterDirectives), and @sentry/nextjs already imports from @sentry/bundler-plugins/core. Could you export this one from core and switch the Next.js loaders to it, so the edge cases you fixed here (CR-only, U+2028/9, ++/--, in/instanceof) apply there too? Happy for that to be a follow-up PR if you'd rather keep this one focused.

[Andarist]

addressed this in 1679219

[timfish]

Nit: this copies the whole remainder of the chunk to test at most a dozen characters. A bounded slice (position + 'instanceof'.length + 1) gives the same result.

[Andarist]

addressed in 1679219#diff-ee8efc138ed4ece111e3e545a52b287db0855ed22d471cf72dc329ed380f9978

[chargome]

q: Can you double check if we still use this one and otherwise delete?

[chargome]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 5f00879. Configure here.}