fix(optimizer): preserve sourcemaps for transformed optimized deps with follow-up transforms

[Andarist]

fixes #13560

On the screenshot above, we can see that root fails to be evaluated to the correct identifier (because it can't be properly mapped). I stumbled upon this in a previous version of Vite that was using esbuild and that was always renaming original identifiers that were shadowing some outer bindings (esbuild playground):

// input
export let root = { outer: 1 };

export function test() {
  let root = { inner: 2 };
  return root;
}

// output
export let root = { outer: 1 };
export function test() {
  let root2 = { inner: 2 };
  return root2;
}

In this PR, I had to figure out some oxc-based builtin transform that would be used by the optimizer that would actually rename some original binding (that's why the tests in this PR don't rely on the above example).

[Andarist]

this is somewhat defensive... but I wasn't sure if you'd prefer a hard crash on invalid data here or how this should otherwise behave

[Andarist]

This is required for the added Babel-based test. When it sees the sourceMappingURL comment it actually grabs that map and does something with the information contained in it. That then somehow conflicts with the Rolldown's sourcemap chaining. I think that mechanism is preferred (transform hook itself doesn't give users access to the sourcemap and the user is not requested to pass the input source map to the performed transform and handle that on their own) - so I think it makes sense to just strip it here to avoid issues with doubly-processed source maps.

[sapphi-red]

I guess we can set sourcemap: 'hidden' here.

vite/packages/vite/src/node/optimizer/index.ts

Line 861 in 98b8163

sourcemap: true,

That should remove the comment.

[Andarist]

Nice! I pushed out this change.

[sapphi-red]

I guess we can set sourcemap: 'hidden' here.

vite/packages/vite/src/node/optimizer/index.ts

Line 861 in 98b8163

sourcemap: true,

That should remove the comment.

[sapphi-red]

Can we merge this to playground/js-sourcemap/vite.config.js?

[Andarist]

I specifically wanted to isolate it because it relies on specific rolldownOptions. I can merge it back into that config but I felt it's kinda harder to grasp what is actually needed for which test there, as a bunch of different tests were running against that "catch-all" config that had to handle all of them at once.

[Andarist]

But ofc if you want to keep them together - I can move this into that existing config

[sapphi-red]

I prefer to be merged for now. The relationship between the tests and the option is not complex. I think we should have a comment on rolldownOptions.transform.target anyways because it's unclear why that is needed.

[Andarist]

Ok, I got them merged.

[Andarist]

@sapphi-red I'm confused as to why zizmor would fail here. I'm not even touching any workflow files and I synced this with the main branch.

[sapphi-red]

You can ignore the zizmor failure. It is failing because the action we are using was taken down temporary due to a recent incident (just to clarify we were / are not affected).