icelam / html-inline-script-webpack-plugin

A webpack plugin for converting external script files to inline script block. Requires 'html-webpack-plugin' to work.
https://www.npmjs.com/package/html-inline-script-webpack-plugin
MIT License
55 stars 10 forks source link

Support preserving the inlined asset #434

Closed SorsOps closed 1 year ago

SorsOps commented 1 year ago

Description

Adds a filter to support whether or not an asset should be deleted from the output.

This has a niche application where we needed to preserve the bundled script in the output to upload to another server in addition to having it inlined.

How has this been tested?

Added a test for it

Types of changes

Remarks

N/A

SorsOps commented 1 year ago

@icelam Let me know if there are any changes needed

icelam commented 1 year ago

Hi @SorsOps, thank you for submitting this Pull Request, your addition of a new feature is appreciated.

How do you feel about changing preserveAsset: (assetName: string) => boolean to assetsPreservePattern: RegExp[]? I understand that using a function allows user to have more control on what assets needs to be preserved. However, consider that both existing parameters htmlMatchPattern and scriptMatchPattern are having type RegExp[], it will be more consistent and intuitive to user that preserveAsset will also accept an array of regex. I did consider changing the existing parameters to accept the feature, but I don't want users to have to tweak their webpack config when they upgrade this plugin.

We might just need to add a utility function that is similar to the existing isFileNeedsToBeInlined to achieve the same result.

SorsOps commented 1 year ago

@icelam Makes sense. I've made the recommended changes

icelam commented 1 year ago

@SorsOps Thank you for updating your PR. The update looks good to me. I just have one minor comment that I would like to rename preserveAsset to assetPreservePattern so that it sync with other variable names. To speed up the process, allow me to co-author this PR. I will release a new version of package once I merge this PR and update the README.

Have a nice weekend :)

icelam commented 1 year ago

@SorsOps I have published v3.2.0 which includes this additional feature! Let me know if it has any issue.