SAFE-Stack / SAFE-template

dotnet CLI template for SAFE project
MIT License
282 stars 87 forks source link

Webpack improvements #491

Closed wtorricos closed 2 years ago

wtorricos commented 2 years ago

I want to propose the following changes to webpack in order to align with common practices in the JS world. Here is the list of changes:

Some additional changes:

I'll be happy to hear your feedback on this proposal, so please let me know if anything can be improved.

closes #488

olivercoad commented 2 years ago

Has anyone tried writing their Webpack config in F# yet? 🤪

wtorricos commented 2 years ago

Has anyone tried writing their Webpack config in F# yet? 🤪

Not yet, but could be interesting. I'm thinking that using anonymous records could be the easiest thing to do.

theimowski commented 2 years ago

Looks good :+1: @Zaid-Ajaj @MangelMaxime can you have a look please?

MangelMaxime commented 2 years ago

All the stuff listed in @tico321 comments seems goods to me.

One tips, that I like to use now days for the plugins array is to use .Filter(Boolean) as I find it easier to read and add new plugins depending on the target:

        plugins:
            [
                new HtmlWebpackPlugin({
                    filename: "./index.html",
                    template: "./src/index.html"
                }),
                new MiniCssExtractPlugin(),
                isProduction && myProductionOnlyPlugin() // This plugin will only be added for production
            ].filter(Boolean),

But it depends on what people find here to read between that and the concat.

About the symlink comment, I suppose it is not required anymore has Fable doesn't retrieve the file via webpack anymore.

olivercoad commented 2 years ago

Yep, I think I like that. That way you don't have to scroll up to find commonPlugins half a screen earlier. Not quite as clean as F# (implicit) yields though 😅. It does use some javascript idiosyncrasies that not everyone might be familiar with, but I think with a comment there it's probably fine.

wtorricos commented 2 years ago

As it looks like we no longer need the symlink I just removed it. I also played a little bit with the filter option that @MangelMaxime suggested and I think it does look cleaner, so I added that modification as well (Note that I also updated the comments and placed them above the plugins). Any feedback is welcome, so please let me know if it looks awkward and I can rollback to use a common plugins variable.

wtorricos commented 2 years ago

Is anything preventing the PR from being merged? Please let me know if I missed something

theimowski commented 2 years ago

Sorry, didn't have time to get to this up till now. This looks awesome - many thanks! There are a few things we still need to wrap up before releasing new template version