infor-cloud / m3-h5-sdk

https://infor-cloud.github.io/m3-h5-sdk/
34 stars 20 forks source link

Don't include unused CSS files in build #118

Open LucasLundJensen opened 2 years ago

LucasLundJensen commented 2 years ago

Is your feature request related to a problem? Please describe. We've ran into issues with brand new H5 applications being over 10MB without any custom written code and M3 doesn't allow larger than 10MB files to be uploaded via Personalization Manager.

Describe the solution you'd like I've made a pull-request: https://github.com/infor-cloud/m3-h5-sdk/pull/114 that basically just changes what files are included in the build.

Describe alternatives you've considered Ontop of my solution, it would be better to use the .min.css files but I'm not sure if that's supposed to be m3-h5-sdk or enterprise-ng that's responsible for that.

anhallbe commented 2 years ago

Good catch! Yes, it would be better to only include minified assets in the zip.

Will look into this when upgrading to newer Angular / IDS, which should decrease the output even further.

dohjon commented 2 years ago

We encountered the same issue and this is how we solved it (fast) for our specific use case.

angular.json

"src/assets",
{
 ...
  "glob": "**/*.min.css",
},
{
 ...
  "glob": "**/*.min.js",
}

index.html

    <script>
      // https://github.com/infor-design/enterprise/pull/2804
      window["SohoConfig"] = { minifyCultures: true };
    </script>

    <link
     ...
      href="assets/ids-enterprise/css/theme-new-light.min.css"
    />
swuendri commented 1 year ago

I'm right, is this related to generated files from odin new? If yes, should we only include theme-new-* css files and exclude classic, soho, uplift files? I would prepare a PR but need your opinion(s). In my opinion PR #114 will not solve the "problem".

LucasLundJensen commented 1 year ago

I'm right, is this related to generated files from odin new? If yes, should we only include theme-new-* css files and exclude classic, soho, uplift files? I would prepare a PR but need your opinion(s). In my opinion PR #114 will not solve the "problem".

They're derived from ids-enterprise which gets "generated" through odin new yes.

I don't know if NOT including the other theme options is the proper solution, as some companies might use them already.

I think the better option is to just include the .min.css files, and use those in index.html by default, instead of bundling the .css and .css.map files.

I agree, in that PR #114 does not solve the solution, it was merely just a quick example of a solution I had used.

swuendri commented 1 year ago

Thank you for your fast and helpful reply @LucasLundJensen!

Maybe an additional option for odin new is the better choice to decide whether all themes should be included (as a default) or only theme-new. I will check this possibility.

tmcconechy commented 1 year ago

Quick note: Some of the theme names are "duplicated" in the node_modules/css folder just for backwards compatibility to older versions. So you should just need

theme-new-contrast
theme-new-dark
theme-new-light

and if you want to support the old look

theme-classic-contrast
theme-classic-dark
theme-classic-light

uplift and soho are the old names for new and classic

swuendri commented 1 year ago

Thank you @tmcconechy for the hint!