mapbox/mapbox-gl-js

When Cache API is not available, dont clone response for `cachePut`

Open

#8,416 opened on Jul 1, 2019

View on GitHub
 (2 comments) (0 reactions) (0 assignees)JavaScript (10,532 stars) (2,203 forks)batch import
bug :lady_beetle:good first issue

Description

The cacheGet method calls back to validateOrFetch with a null error if window.caches is not available. From what I can tell, this could happen when the Cache API is not available at all or if a page is served over http and the Cache API is not usable.

https://github.com/mapbox/mapbox-gl-js/blob/dd5dcd025a5465e85330ac564a27d3a4b60839dc/src/util/tile_request_cache.js#L72-L81

In this case validateOrFetch still calls clones the response and attempts to place it in the cache.

https://github.com/mapbox/mapbox-gl-js/blob/dd5dcd025a5465e85330ac564a27d3a4b60839dc/src/util/ajax.js#L127-L131

If it is known that window.caches is no available or usable, there is not need to clone the response or to load its contents:

https://github.com/mapbox/mapbox-gl-js/blob/dd5dcd025a5465e85330ac564a27d3a4b60839dc/src/util/ajax.js#L144-L152

Contributor guide

Tech stack
javascript
Domain
frontendperformance
Issue type
performance
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
mostly clear
Prerequisites
basic JavaScriptunderstanding of Cache API
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
60
Research direction
Look at src/util/tile request cache.js lines 72-81 and src/util/ajax.js lines 127-152. The issue suggests that when window.caches is unavailable, the response should not be cloned. Check if there is a condition to skip cloning when cache is unavailable. The fix involves adding a check before cloning in the validateOrFetch function or modifying the cachePut call to avoid unnecessary work.