search

fusionjs / fusion-cli

Migrated to https://github.com/fusionjs/fusionjs
MIT License
140 stars 37 forks source link

Change webpack dev sourcemap strategy to cheap-eval-source-map #758

Open derekjuber opened 5 years ago

derekjuber commented 5 years ago

Addresses WPT-1864.

Our previous source map strategy was not bundling all of the individual source map files in node_modules into the final vendor.js.map file. By my rough estimations less than 25% of our fusion plugins were being added to the final generated mapping file. I cannot say why as attempting to track down the reasons didn't really lead anywhere after hours of learning about how this stuff works.

I started investigating alternative source map strategies and basically any strategy that allows source map debugging in original code would break. Anything else that uses generated or transformed code was fine though. So I picked the best one for dev based on that with fast build and rebuild speeds.

image image image

Our previous pick was:

image image image

Going by the chart it seems like our previous pick wasn't the best? Webpack doesn't recommend it for dev or production but only for special situations which I am unaware of.

An alternative approach would be to figure out why original source source maps aren't working. The documentation says this depends on loader support so something could be wrong with our loaders. I'm ok with going down that route if we don't want to settle with transpiled code source maps.

Personally it doesn't bother me that much since I can recognize what the code is doing even if it was transpiled but I can see the arguments for preserving the original code for maximum debugability.

Update: Forgot to add that this does remove the external js.map file for dev and instead inlines all of the source mappings into the generated vendor.js bundle. File size goes from about 2 MB to 5 MB and beyond. I don't see this as an issue in dev though since this is all local.

rtsao commented 5 years ago

For what it's worth, both create-react-app and Next.js both use cheap-module-source-map.

My understanding is that source maps with webpack is still something of an unsolved problem in that no option really works perfectly with Chrome DevTools.

Our previous source map strategy was not bundling all of the individual source map files in node_modules into the final vendor.js.map file.

Are you referring to the source map files already in node_modules such as: ./fusion-plugin-rpc/dist/browser.es2017.es.js.map?

Honestly, I'm not even totally sure we'd want to consume those node_module source maps in Fusion, since the actual code bundled by webpack would be ./dist/browser.es2017.es.js and not ./src/xyz.js.

derekjuber commented 5 years ago

@rtsao: Hmm. The fact that the other two frameworks use cheap-module-source-map does give me some pause. Though it looks like there was an effort to change create-react-app to something else but it was reverted due to unfortunate Chrome and Safari issues.

See https://github.com/facebook/create-react-app/pull/4930 and https://github.com/facebook/create-react-app/pull/5059.

Do we not want to ingest node module source map files? I probably need to dig through the splitChunks source code but the way I thought this would work for the vendor bundle would be that Webpack crawls through node_modules, concats all of the dist/ files and then concats the corresponding source map file into the vendor js.map file (if it exists, I guess it would have to manually create the mappings for node modules that do not distribute the files).

I'll take another pass at this but cheap-eval-source-map worked great when I tested this but what I couldn't confirm was if this broke something else.