Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

Add support for async loading the objectFitPolyfill script #49

Merged
merged 1 commit into from
Jun 27, 2019

Conversation

erinsinger93
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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

@cee-chen cee-chen merged commit 1dc7ce4 into cee-chen:master Jun 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants