✨[amp-story-player] Add support to load documents from the cache#27658
✨[amp-story-player] Add support to load documents from the cache#27658Enriqe merged 3 commits intoampproject:masterfrom
Conversation
kristoferbaxter
left a comment
There was a problem hiding this comment.
Small items to consider.
| } | ||
|
|
||
| if (!SUPPORTED_CACHES.includes(ampCache)) { | ||
| throw new Error( |
There was a problem hiding this comment.
Did we discuss if we want to fallback on the origin or completely fail?
There was a problem hiding this comment.
We did discuss and I believe the consensus was that we shouldn't fallback to origin.
/cc @newmuis
* updating amp-toolbox for newly introduced servingType support * updates PR with recent revert from custom elements
Enriqe
left a comment
There was a problem hiding this comment.
Updated the PR with the recent changes from moving the code to another directory, adding support in the amp-toolbox package for servingType, etc.. PTAL :)
|
/to @kristoferbaxter for owners approval |
|
Friendly ping @kristoferbaxter |
danielrozenberg
left a comment
There was a problem hiding this comment.
package.json approval
…project#27658) * load documents from the cache when speciifed by publisher * get cache options from caches.js, wait for cacheurl promise * update PR with recent changes: * updating amp-toolbox for newly introduced servingType support * updates PR with recent revert from custom elements
| * limitations under the License. | ||
| */ | ||
|
|
||
| import * as ampCaches from '../../build-system/global-configs/caches.json'; |
There was a problem hiding this comment.
Importing from build-system isn't supported, because isn't not included as one of our files when we pass them to closure.
Additionally, you can't import JSON like this in real JS, this is a node behavior without real semantics. TC39 is currently discussing JSON imports, but semantics we have will only expose a default export:
import ampCaches from '…/caches.json' with type: 'json';
ampCaches.caches.map(…)I'd imagine closure will have undefined semantics for this in the meantime. We'd need to preprocess this file into a JS file during builds, and you can import from there.
Closes #27588