search

umbraco / Umbraco.Forms.Issues

Public issue tracker for Umbraco Forms
29 stars 0 forks source link

Request: Remove Promise Polyfill #1155

Closed Nicholas-Westby closed 5 months ago

Nicholas-Westby commented 5 months ago

I noticed that when I render Umbraco Forms (version 13.0.1) dependencies like this:

@Html.RenderUmbracoFormDependencies(Url)

The HTML that results is the following (whitespace added for clarity):

<script src="/App_Plugins/UmbracoForms/Assets/promise-polyfill/dist/polyfill.min.js" type="application/javascript"></script>
<script src="/App_Plugins/UmbracoForms/Assets/aspnet-client-validation/dist/aspnet-validation.min.js" type="application/javascript"></script>

It looks like that first script is a promise polyfill.

Given that promises are now widely supported across all modern browsers (IE is irrelevant at this point): https://caniuse.com/promises

I recommend removing this from future versions so that you no longer incorporate the promise polyfill. Someone can still add one if they want, but it doesn't seem like there is much value in including it as part of the core Umbraco Forms offering, and now only really serves to lower page speed.

AndyButland commented 5 months ago

Thanks @Nicholas-Westby - I agree this could well have been removed some time ago, but I guess strictly speaking doing it now would be a breaking change, so I think we'll have to leave it for the next major (which we have started work on, so I've made the change in the working branch there).

I did consider if we could add a configuration option so you could switch off rendering this polyfill in existing versions. But as RenderUmbracoFormDependencies is a static method, there's not really a clean way to use these options without changing the signature of the method. And it is of course possible to just not use this HTML helper and render just the scripts you do need instead.