Address #27 - Dedupe in-flight requests and add 2 minute stylesheet caching

[pl12133]

To address the duplicated requests, I added deduplication and caching to loadExternal.

Using the awesome minimal test case from @Frxnz, I saw 3 XHR requests for each stylesheet. Here is a fixed test case showing only 1 request by loadExternal for each stylesheet.

I was unable to run your test suite because I could not find the replace package on Arch Linux, sorry. :)

[developit]

No arrow functions here 🦀

[ausi]

Thank you for this pull request!

I was unable to run your test suite because I could not find the replace package on Arch Linux, sorry. :)

Nothing to be sorry about, I guess I have to update the devDependencies in the package.json sometime :)

I modified your code a little bit in PR #36, combined CACHE and IN_FLIGHT to requestCache and renamed CACHE_MAX_AGE to requestCacheTime. And I added a test case for the deduplication.

I’m not sure if the 2 minute cache timeout is actually necessary. Can you imagine a case where it is needed?

[developit]

@ausi VS no expiry? I think it might be useful for something like dynamically updated stylesheets, but at least in the case I'm thinking of (webpack dev server), the URL actually changes.

Context: we are using cq-prolyfill on a fairly large website with a lot of dynamically injected links to stylesheets. Right now reevaluate() is re-fetching and parsing a pretty huge number of stylesheets, enough that we noticed it in network graphs haha.

[pl12133]

Thank you for reviewing and cleaning up the code, and for adding supporting tests. @developit covered the use case here - upon page loads we are seeing a single stylesheet being requested about 20~ times over the course of roughly 4 seconds.

The caching is something I usually include as standard for repeated AJAX requests in order to avoid network calls as much as possible. I just tried it out without caching, and I still saw repeated requests for the same stylesheet. 1 second after the first de-duplicated request had returned, a repeated request for the same stylesheet was made. So because of that, I think we need at least a short cache here to solve #27. Maybe the expire time could come down to 30 seconds?

[ausi]

Maybe the expire time could come down to 30 seconds?

I think we could remove the expiration completely, meaning cache it for the whole lifetime of the page.

[developit]

Agreed - at first I had though you were asking about reducing the cache time @ausi, but it makes sense to cache infinitely - it's in-memory, and any network call to the same URL in that same context is going to get the same stylesheet anyway. At least for our use-case, in-memory caching without expiry should cover all needs.

[pl12133]

Then I agree too there's no need for an expiration. I also thought @ausi was looking for a shorter cache time.

[ausi]

Great! I updated #36.

After the BrowserStack API starts working again and the tests pass in all browsers, I will merge it.

[pl12133]

Thanks for the quick follow up and the cool project!