fix: sourcemap error with plugin-react #5563

sanyuan0704 · 2021-11-05T14:03:41Z

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 · 2021-11-05T14:11:11Z

@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 · 2021-11-05T14:14:14Z

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

sanyuan0704 · 2021-11-05T14:20:06Z

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 · 2021-11-05T14:36:55Z

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 · 2021-11-05T15:27:24Z

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 · 2021-11-05T15:37:38Z

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 · 2021-11-05T15:47:07Z

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 · 2021-11-05T20:32:57Z

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 · 2021-11-06T01:20:42Z

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 · 2021-11-06T01:31:09Z

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 · 2021-11-06T03:43:33Z

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 · 2021-11-06T06:09:30Z

@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 · 2021-11-06T07:58:51Z

@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 · 2021-11-07T16:56:22Z

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 · 2021-11-08T00:50:43Z

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 · 2021-11-08T01:00:12Z

@antfu PTAL

sanyuan0704 · 2021-11-08T01:42:58Z

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

aleclarson · 2021-11-08T17:52:10Z

We're going with #5255 and #5587 instead

sanyuan0704 mentioned this pull request Nov 5, 2021

plugin-react 1.0.6 triggers sourcemap and deoptimisation error/warning messages in dev server #5438

Closed

7 tasks

sanyuan0704 force-pushed the fix/react-sourcemap branch from 6725937 to 14a7985 Compare November 5, 2021 14:17

sanyuan0704 force-pushed the fix/react-sourcemap branch from 14a7985 to 888241a Compare November 5, 2021 14:30

Shinigami92 requested a review from aleclarson November 5, 2021 14:41

Shinigami92 added plugin: react p3-minor-bug An edge case that only affects very specific usage (priority) labels Nov 5, 2021

sanyuan0704 force-pushed the fix/react-sourcemap branch from 888241a to 455cc7e Compare November 5, 2021 14:42

fix: sourcemap error with plugin-react

16d0833

sanyuan0704 force-pushed the fix/react-sourcemap branch from 455cc7e to 16d0833 Compare November 5, 2021 14:43

aleclarson suggested changes Nov 5, 2021

View reviewed changes

cymen mentioned this pull request Nov 5, 2021

v0.1.3 Sourcemap for "..node_modules/.vite/*.js" points to missing source files storybookjs/builder-vite#129

Closed

templeman15 referenced this pull request in ElMassimo/vite_ruby Nov 5, 2021

fix: avoid generating sourcemaps when running tests

2b4faac

fix: include sourcesContent after remapping

e689446

sanyuan0704 force-pushed the fix/react-sourcemap branch from 55c8609 to e689446 Compare November 6, 2021 03:40

frandiox mentioned this pull request Nov 8, 2021

[BUG] RSC client components displaying warning in Vite dev console Shopify/hydrogen-v1#140

Closed

sanyuan0704 requested a review from aleclarson November 8, 2021 11:57

aleclarson closed this Nov 8, 2021

johnson86tw mentioned this pull request Dec 19, 2022

Vite dev server warning: Sourcemap for "xxxx" points to missing source files #5562 niksy/node-stdlib-browser#25

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: sourcemap error with plugin-react #5563

fix: sourcemap error with plugin-react #5563

sanyuan0704 commented Nov 5, 2021 •

edited

Loading

sanyuan0704 commented Nov 5, 2021 •

edited

Loading

Niputi commented Nov 5, 2021 •

edited

Loading

sanyuan0704 commented Nov 5, 2021

sanyuan0704 commented Nov 5, 2021

bluwy commented Nov 5, 2021

sanyuan0704 commented Nov 5, 2021

sanyuan0704 commented Nov 5, 2021

aleclarson commented Nov 5, 2021

aleclarson left a comment

sanyuan0704 commented Nov 6, 2021 •

edited

Loading

sanyuan0704 commented Nov 6, 2021 •

edited

Loading

sanyuan0704 commented Nov 6, 2021

bluwy commented Nov 6, 2021

sanyuan0704 commented Nov 6, 2021

templeman15 commented Nov 7, 2021

sanyuan0704 commented Nov 8, 2021

sanyuan0704 commented Nov 8, 2021

sanyuan0704 commented Nov 8, 2021

aleclarson commented Nov 8, 2021

fix: sourcemap error with plugin-react #5563

fix: sourcemap error with plugin-react #5563

Conversation

sanyuan0704 commented Nov 5, 2021 • edited Loading

Description

Additional context

What is the purpose of this pull request?

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

sanyuan0704 commented Nov 5, 2021 • edited Loading

Niputi commented Nov 5, 2021 • edited Loading

sanyuan0704 commented Nov 5, 2021

sanyuan0704 commented Nov 5, 2021

bluwy commented Nov 5, 2021

sanyuan0704 commented Nov 5, 2021

sanyuan0704 commented Nov 5, 2021

aleclarson commented Nov 5, 2021

aleclarson left a comment

Choose a reason for hiding this comment

sanyuan0704 commented Nov 6, 2021 • edited Loading

sanyuan0704 commented Nov 6, 2021 • edited Loading

sanyuan0704 commented Nov 6, 2021

bluwy commented Nov 6, 2021

sanyuan0704 commented Nov 6, 2021

templeman15 commented Nov 7, 2021

sanyuan0704 commented Nov 8, 2021

sanyuan0704 commented Nov 8, 2021

sanyuan0704 commented Nov 8, 2021

aleclarson commented Nov 8, 2021

sanyuan0704 commented Nov 5, 2021 •

edited

Loading

sanyuan0704 commented Nov 5, 2021 •

edited

Loading

Niputi commented Nov 5, 2021 •

edited

Loading

sanyuan0704 commented Nov 6, 2021 •

edited

Loading

sanyuan0704 commented Nov 6, 2021 •

edited

Loading