futtta / autoptimize

Official Autoptimize repo on Github
https://autoptimize.com/pro/
GNU General Public License v2.0
283 stars 87 forks source link

Make compatibility configurable #384

Closed fliespl closed 2 years ago

fliespl commented 2 years ago

Sample case:

Of course I can exclude it, but maybe making "compatibility" class loaded based by option seems better way?

fliespl commented 2 years ago

Additionally two things :)

  1. I think autoptimizeCompatibility class should add exclusion to both: jquery.min.js & jquery.js

  2. Also it would be best if this could recognize DOMContentLoaded jquery scripts (probably also window load event?):

<script type="text/javascript">document.addEventListener("DOMContentLoaded", function () {
jQuery(function($) { 
// something
});
});
</script>

Those are executions that will happen after jQuery is loaded - so it could be minifed.

futtta commented 2 years ago

You mean an "enhanced compatibility" option which auto-excludes jQuery ?

On 22/05/2022 22:21, fliespl wrote:

Sample case:

  • jQuery was not excluded on 2.9
  • Updated to 3+ (autoptimize_installed_before_compatibility) was updated to false due to upgrade
  • New plugin was installed which added i.e. jQuery inlined script

Of course I can exclude it, but maybe making "compatibility" class loaded based by option seems better way?

— Reply to this email directly, view it on GitHub https://github.com/futtta/autoptimize/issues/384, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABMIMPE57XKVPIGNE3WOA3VLKJNTANCNFSM5WT36YHA. You are receiving this because you are subscribed to this thread.Message ID: @.***>

fliespl commented 2 years ago

Yes, I think that makes sense.

futtta commented 2 years ago

OK, took me some time (I'm slow on Mondays) but I think I implemented this with https://github.com/futtta/autoptimize/commit/1a8ec3d466bc6ca2d311e78432e13bf100f85149 and https://github.com/futtta/autoptimize/commit/e538520aa22913d0b6a3224b7a5b61b9df2dc65b. The domcontentloaded one is a bit more tricky to get right though, so I'll probably "forget" about that (unless you would have a great PR for that ;-) ). Would be great if you could test these changes?

futtta commented 2 years ago

have you been able to have a look at those changes/ test them by any chance @fliespl

fliespl commented 2 years ago

Yeah, I have tried that, but it really is more complicated :) Especially since items can be mixed (domcontentloaded function closed, while other jquery is appended after), I think it might end up with plenty of issues so probably worth dropping this idea.

futtta commented 2 years ago

It indeed is impossible to take all variations into account, but also auto-excluding jquery.js (which is pretty harmless) and allowing pre-3.0-installed instances to enable the compatibility logic does have some merit so I'll probably keep those in :-)

thanks!

fliespl commented 2 years ago

Oh yes - those changes are cool and work :) Was simply talking about domcontentloaded.

Thanks!