fix: sourcemap error with plugin-react

[sanyuan0704]

Description

Fix sourcemap error when using @vitejs/plugin-react，see details in #5438, #5562

Additional context

The product of pre-bundle is incorrectly transformed by babel in plugin-react.This led to a series of unpredictable sourcemap problems, because this plugin will generate some sourcemaps which will be push into vite's sourcemapChain and combined after transform hook.

// pluginContainer.ts
async transform(code, id, options) {
  const inMap = options?.inMap
  const ssr = options?.ssr
  const ctx = new TransformContext(id, code, inMap as SourceMap)
  ctx.ssr = !!ssr
  for (const plugin of plugins) {
    if (!plugin.transform) continue
    let result: TransformResult | string | undefined
    try {
      result = await plugin.transform.call(ctx as any, code, id, { ssr })
    } catch (e) {
      ctx.error(e)
    }
    if (isObject(result)) {
      code = result.code || ''
      // push into sourcemapChain
      if (result.map) ctx.sourcemapChain.push(result.map)
    } else {
      code = result
    }
  }
  return {
    code,
    // combine the sourcemaps
    map: ctx._getCombinedSourcemap()
  }
},

The reason why the 1.0.6 plugin version does not have this problem is because of the
request of pre-bundle deps will carry the querystring suffix of BrowserHash, thus skipping the transform process of react:babel. But in 1.0.7 version the plugin filters out the querystring suffix, so that the pre-bundle result such as /xxx/node_modules/.vite/react.js also enters the transform of babel which is unespect.

Now the solution is as follows:

• For the pre-bundled result, skip babel's transform to avoid huge performance consumption as well as the warning of Babel:

[BABEL] Note: The code generator has deoptimised the styling of /Users/sanyuan/react-project/node_modules/.vite/react-dom.js?v=768d5ed4 as it exceeds the max of 500KB.

• For warning about SourceMap，i found out sourceContent is excluded in remapping process.It can be solved by including sourcesContent after remapping if additional soucemap is added after transform hook.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[sanyuan0704]

@Niputi @bluwy @Shinigami92 @patak-js PTAL. I think it will be a serious issue because the plugin will use babel to transform pre-bundled result which is very large.And there will be a lot of error messages in the console, such as

"Sourcemap for "xxxx" points to missing source files"

This is bound to affect DX very much.

[Niputi]

tests need to be passing.
@aleclarson is working on a more complete solution for react sourcemap issue

[sanyuan0704]

tests need to be passing. @aleclarson is working on a solution for react sourcemap issue

ok，i will adjust to pass the test. @aleclarson PTAL

[sanyuan0704]

This issue has existed for more than a week. And the 1.0.7 version was proposed at the time, and now the 1.1.0 beta version, the problem still exists.I will be very grateful if I can help solve this problem.

[bluwy]

FWIW I've encountered this sourcemap warning too in vite-plugin-svelte using the prebundleSvelteLibraries experimental option, which in brief, works by compiling Svelte files during the prebundling phase (same situation in this PR).

I've been debugging this myself to no avail, but I think Vite should work even if we return sourcemaps during the prebundling phase. Maybe that's the long term solution, but I'd be interested to see if anyone knows why, instead of skipping transformation in prebundling.

[sanyuan0704]

It’s not just the sourcemap problem. Using babel to transform the pre-bundled dep in this plugin is a very performance-consuming and unnecessary thing originally.And The page load time will be very long. This is one of the reasons why I proposed this solution.

[sanyuan0704]

FWIW I've encountered this sourcemap warning too in vite-plugin-svelte using the prebundleSvelteLibraries experimental option, which in brief, works by compiling Svelte files during the prebundling phase (same situation in this PR).

I've been debugging this myself to no avail, but I think Vite should work even if we return sourcemaps during the prebundling phase. Maybe that's the long term solution, but I'd be interested to see if anyone knows why, instead of skipping transformation in prebundling.

Regarding the problem of souremap, I very much agree with you, and I feel the same way. I think that Vite should also be able to work even when additional sourcemaps are added. I will try to find a way to solve it inside vite.That's another matter.

[aleclarson]

Using babel to transform the pre-bundled dep in this plugin is a very performance-consuming and unnecessary thing

It is necessary, because we want dev to match production as closely as possible, and that means applying the "automatic JSX runtime" transform to node_modules. In your Vite config, you can set jsxRuntime: "classic" when configEnv.mode === 'development' if you don't mind the difference between dev and production.

[aleclarson]

I don't think this is the right fix. We should instead make sure the sourcemaps generated by pre-bundling don't include sources like dep:react or we should tolerate such sources wherever sourcemaps are processed by Vite.

[sanyuan0704]

Using babel to transform the pre-bundled dep in this plugin is a very performance-consuming and unnecessary thing

It is necessary, because we want dev to match production as closely as possible, and that means applying the "automatic JSX runtime" transform to node_modules. In your Vite config, you can set jsxRuntime: "classic" when configEnv.mode === 'development' if you don't mind the difference between dev and production.

If you really try it in a real project, you will find that using babel transfom will be very slow, and in general projects there will be a large number of 500 KB pre-bundle results (this will also appear a series of waring about babel, which confused the users). See detail in #5438.

[BABEL] Note: The code generator has deoptimised the styling of /Users/sanyuan/react-project/node_modules/.vite/react-dom.js?v=768d5ed4 as it exceeds the max of 500KB.

These large results are packaged with babel one by one, and it is inevitable that the page will take twenty seconds or more to open. This is horrible. I don’t think it’s a must to use babel in the development phase, or it can’t be used as a built-in default operation.

[sanyuan0704]

I don't think this is the right fix. We should instead make sure the sourcemaps generated by pre-bundling don't include sources like dep:react or we should tolerate such sources wherever sourcemaps are processed by Vite.

We need to make some balance between DX and development-production environment consistency. In this more extreme situation, I think we need to give priority to DX, because fast is originally a core advantage of Vite. To use babel to transform deps, I think it can be left to the user to configure and decide which deps are needed transforming, instead of all the dependency code transformed by default.

[sanyuan0704]

FWIW I've encountered this sourcemap warning too in vite-plugin-svelte using the prebundleSvelteLibraries experimental option, which in brief, works by compiling Svelte files during the prebundling phase (same situation in this PR).

I've been debugging this myself to no avail, but I think Vite should work even if we return sourcemaps during the prebundling phase. Maybe that's the long term solution, but I'd be interested to see if anyone knows why, instead of skipping transformation in prebundling.

@bluwy I have found out why additional sourcemap couldn't work in vite.In the logic of combineSourceMap, remapping function is called, but the sourcesContent is excluded in the end, which leads to the original file not being found in vite. I have updated the code.

[bluwy]

@bluwy I have found out why additional sourcemap couldn't work in vite.In the logic of combineSourceMap, remapping function is called, but the sourcesContent is excluded in the end, which leads to the original file not being found in vite. I have updated the code.

Thanks for looking into this! I tested it locally again and it seems to still report the same sourcemap warnings. Maybe it's because that Svelte generates a relative source file name, but I'll try to look into that myself. Regarding the remapping change, I'm not a sourcemap wizard so I can't tell if it's a desirable change or not 😅

[sanyuan0704]

@bluwy I have found out why additional sourcemap couldn't work in vite.In the logic of combineSourceMap, remapping function is called, but the sourcesContent is excluded in the end, which leads to the original file not being found in vite. I have updated the code.

Thanks for looking into this! I tested it locally again and it seems to still report the same sourcemap warnings. Maybe it's because that Svelte generates a relative source file name, but I'll try to look into that myself. Regarding the remapping change, I'm not a sourcemap wizard so I can't tell if it's a desirable change or not 😅

I haven't tried Svelte, but this solution can solve the sourcemap warning problem under the React project. I try to solve it in svelte, maybe I can mention another PR.

[templeman15]

These large results are packaged with babel one by one, and it is inevitable that the page will take twenty seconds or more to open. This is horrible. I don’t think it’s a must to use babel in the development phase, or it can’t be used as a built-in default operation.

I'm experiencing this now in a monolith Rails app with React and some legacy haml views, scss and Svelte. Before this started happening I was able to see the browser load in about 20 seconds. Now it takes 60-90 seconds before the browser actually loads. I spent a significant amount of time getting us switched over to Vite and now my team is less then thrilled about this behavior and wasn't able to experience the quick load times like I was. The warnings are alarming and the time to wait for the browser to load is almost not acceptable for most of my team.

Thank you for looking into this!

[sanyuan0704]

These large results are packaged with babel one by one, and it is inevitable that the page will take twenty seconds or more to open. This is horrible. I don’t think it’s a must to use babel in the development phase, or it can’t be used as a built-in default operation.

I'm experiencing this now in a monolith Rails app with React and some legacy haml views, scss and Svelte. Before this started happening I was able to see the browser load in about 20 seconds. Now it takes 60-90 seconds before the browser actually loads. I spent a significant amount of time getting us switched over to Vite and now my team is less then thrilled about this behavior and wasn't able to experience the quick load times like I was. The warnings are alarming and the time to wait for the browser to load is almost not acceptable for most of my team.

Thank you for looking into this!

@aleclarson PTAL

[sanyuan0704]

@antfu PTAL

[sanyuan0704]

@patak-js PTAL.It seems a serious bug.

[aleclarson]

We're going with #5255 and #5587 instead