fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.88k stars 295 forks source link

Why lib.Long.js and lib/big.js are included in the bundle when using React.useState ? #2907

Closed MangelMaxime closed 1 year ago

MangelMaxime commented 2 years ago

Description

The problem described in this issue occured for me when using React.useState from Feliz but I think this is related to Fable internals that's why I open the issue here. If needed, we can move it to Feliz.

I have a an empty React app consisting in:

module Demo.Hook

open Feliz
open Browser.Dom
open Fable.Core.JsInterop

// Workaround to have React-refresh working
// I need to open an issue on react-refresh to see if they can improve the detection
emitJsStatement () "import React from \"react\""

[<ReactComponent>]
let private Component () =
    let isLeftPanelVisible, setLeftPanelVisibility = React.useState true

    Html.div [
    ]

ReactDOM.render(
    Component ()
    ,
    document.getElementById("root")
)

When building it for production using Vite, I see that big.js and long.js are included in the bundle and impact it by "a lot".

stats_with_long_big.zip

If I remove the line:

let isLeftPanelVisible, setLeftPanelVisibility = React.useState true

then big.js and long.js are not more included.

image

stats_without_long_big.zip

Related information

alfonsogarciacaro commented 2 years ago

Looking at the stats it seems that big.js > Decimal.js is being imported by Reflection while Long.js is being imported by Date.js and Feliz/Interop.js. There are a couple of methods in Date.js that use long (to create dates from ticks or milliseconds) but they don't seem to be used by Feliz/Interop.js.

IIRC a similar thing was observed in the past: when a file was included all dependencies of that file were included too even if they were not used. I think we fixed it for Webpack by adding "sideEffects": false to package.json within fable-library (see this), but maybe this doesn't work the same for Vite/Rollup? It would be interesting to compare the results with Webpack.

inosik commented 2 years ago

The equality operator (or other comparison constructs) can trigger this sometimes as well, because the equals function from Utils.ts depends on a bunch of other things.

Does your bundler have something to view the dependency graph? Then we could track this down.

MangelMaxime commented 2 years ago

The bundler I am using is Vite which use rollup behind the hood.

I found this plugin https://www.npmjs.com/package/rollup-plugin-visualizer that I used to make the charts / stats from above.

inosik commented 2 years ago

Can you generate one of the other artifacts? The "Network" option looks like it's what's we're looking for.

alfonsogarciacaro commented 2 years ago

@MangelMaxime It'd be nice if you could make a bundle with Webpack and see if the results are different. If Webpack manages to remove big.js and long.js dependencies maybe we can assume it's Rollup not understanding "sideEffects": false. In that case, we can consider putting the methods that require Decimal and Long in different files so we're sure the dependencies aren't activated unless those specific methods are called.

MangelMaxime commented 2 years ago

Oops, I though I had provided a reproduction code for this issue but I think I made the Gitpod template and stopped here 🤣

I will, provide a reproduction code so we can apply any bundler to the project for reproduction purpose.

MangelMaxime commented 2 years ago

Here is a reproduction repository https://github.com/MangelMaxime/fable-2907-reproduction

Both Vite and Webpack are setup with a plugin visualize. You can run ./build.sh BuildVite or ./build.sh BuildWebpack depending on which one you want to observe.

alfonsogarciacaro commented 2 years ago

Thanks a lot @MangelMaxime! I modified a bit your example to use webpack-bundle-analyzer which works a bit better and split the code from node_modules (basically react in this case) into a separate chunk. I also modified a bit the code to make sure the bundle contained code from Date module to see if this also brought Long.js in. Webpack generated an app.js bundle of 3.66KB (non-gzipped) which means tree shaking is working pretty well as it's removing all the stuff that's not necessary for that minimal app.

image

It looks like Webpack still has better bundle optimization capabilities than Vite/Rollup after all.