search

cee-chen / object-fit-polyfill

A Javascript polyfill for browsers that don't support the object-fit CSS property.
ISC License
496 stars 93 forks source link

Add support for async loading the objectFitPolyfill script #49

Closed erinsinger93 closed 5 years ago

erinsinger93 commented 5 years ago

Hello! I was wondering if we could change the strategy for adding the event listeners/invoking the objectFitPolyfill to support async loading this script. Otherwise, when this script is loaded async, the DOMContentLoaded hook has already been fired (https://developer.mozilla.org/en-US/docs/Web/API/Document/DOMContentLoaded_event#Checking_whether_loading_is_already_complete)

Thanks!

cee-chen commented 5 years ago

Thanks so much for updating this and for the fantastic idea Erin! Just to confirm, these are the two use cases for async loading that you're thinking of, right? (Feel free to let me know if one is incorrect or if I'm missing any cases!)

  1. When a developer dynamically inserts a <script> tag into the DOM (or a dynamic @import statement, whenever that gets supported :)

  2. A static site that has a script tag loaded on the page, with async or defer attributes

Just want to make sure I have that figured out for future manual testing. Thanks much!

erinsinger93 commented 5 years ago

Yep! The two use cases you've mentioned are what I was thinking of. Case number 2 in your comment is what inspired me to make this PR. I have all of the JS in my app is bundled together and loaded with a script using the async attribute.

While I could have called objectFitPolyfill() in one of my other JS files to ensure the polyfill is eventually applied, it felt like the wrong separation of concerns because the async behavior is happening in my HTML file via the async script tag.

I agree that if, say, I'd lazily fetched more images via JS, then it could definitely be advisable to call objectFitPolyfill() after fetching the images to be more transparent about when the polyfill needs to be reapplied.

Hope that clears up any confusion!

Thanks, Erin

cee-chen commented 5 years ago

Awesome, that 100% clears up any uncertainty I had! Thanks so much for this Erin, this is an amazing PR! 🙇‍♀