simplify fetch expectations and make more consistent with xhr

ampproject · Jan 27, 2020 · daeeacb · daeeacb
1 parent 3445016
commit daeeacb
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 21 deletions.
diff --git a/examples/bind/basic.amp.html b/examples/bind/basic.amp.html
@@ -44,9 +44,9 @@ <h3>"amp-state:" url example:</h3>
 
   <amp-state id="myState">
     <script type="application/json">
-      {
+      items: {
         "myStateKey1": "myStateValue1",
-        "animals": ["okapis", "bears", "pigs"]
+        "animals": {items: ["okapis", "bears", "pigs"]}
       }
     </script>
   </amp-state>

diff --git a/extensions/amp-list/0.1/amp-list.js b/extensions/amp-list/0.1/amp-list.js
@@ -264,11 +264,6 @@ export class AmpList extends AMP.BaseElement {
       this.initializeLoadMoreElements_();
     }
 
-    const src = this.element.getAttribute('src');
-    if (this.isAmpStateSrc_(src)) {
-      return this.renderAmpStateJson_(src);
-    }
-
     return this.fetchList_();
   }
 
@@ -364,14 +359,14 @@ export class AmpList extends AMP.BaseElement {
   }
 
   /**
-   * Render an amp-list that has an "amp-state:" uri. For example,
+   * Gets the json an amp-list that has an "amp-state:" uri. For example,
    * src="amp-state:json.path".
    *
    * @param {string} src
-   * @return {Promise<void>}
+   * @return {Promise<!JsonObject>}
    * @private
    */
-  renderAmpStateJson_(src) {
+  getAmpStateJson_(src) {
     return Services.bindForDocOrNull(this.element)
       .then(bind => {
         userAssert(bind, '"amp-state:" URLs require amp-bind to be installed.');
@@ -388,14 +383,7 @@ export class AmpList extends AMP.BaseElement {
           typeof json !== 'undefined',
           `[amp-list] No data was found at provided uri: ${src}`
         );
-
-        const array = /** @type {!Array} */ (isArray(json) ? json : [json]);
-        return this.scheduleRender_(array, /* append */ false);
-      })
-      .catch(error => {
-        this.triggerFetchErrorEvent_(error);
-        this.showFallback_();
-        throw error;
+        return json;
       });
   }
 
@@ -615,7 +603,10 @@ export class AmpList extends AMP.BaseElement {
     if (this.ssrTemplateHelper_.isEnabled()) {
       fetch = this.ssrTemplate_(opt_refresh);
     } else {
-      fetch = this.prepareAndSendFetch_(opt_refresh).then(data => {
+      fetch = this.isAmpStateSrc_(elementSrc)
+        ? this.getAmpStateJson_(elementSrc)
+        : this.prepareAndSendFetch_(opt_refresh);
+      fetch = fetch.then(data => {
         // Bail if the src has changed while resolving the xhr request.
         if (elementSrc !== this.element.getAttribute('src')) {
           return;

diff --git a/extensions/amp-list/0.1/test/test-amp-list.js b/extensions/amp-list/0.1/test/test-amp-list.js
@@ -1264,7 +1264,7 @@ describes.repeated(
 
             it('should render a list using local data', async () => {
               toggleExperiment(win, experimentName, true);
-              bind.getState = () => [1, 2, 3];
+              bind.getState = () => ({items: [1, 2, 3]});
 
               const ampStateEl = doc.createElement('amp-state');
               ampStateEl.setAttribute('id', 'okapis');
@@ -1275,8 +1275,10 @@ describes.repeated(
 
               listMock
                 .expects('scheduleRender_')
-                .withExactArgs([1, 2, 3], /*append*/ false)
+                .withExactArgs([1, 2, 3], /*append*/ false, {items: [1, 2, 3]})
+                .returns(Promise.resolve())
                 .once();
+
               await list.layoutCallback();
             });
           });