1

The problem I have is that when I click on the "NewQuoteButton" on the FIRST time the page loads, I get an error: quoteGenerator.getQuote(...).then is not a function. However, if I click the button again, I'm able to generate the quotes normally. Also upon examining my network tab in Chrome inspector, I noticed I had a lot more get requests and responses from the API while my quote that is being displayed is lagging behind.

I'm new to Promises and currently practicing utilizing it along with revealing module pattern. My understanding is that a Promise calls then() which takes a function with that resolved promise's value as the argument. Therefore, my getQuote() should work because it returns a Promise containing a quote object. Can someone point me in the right direction. Thanks!

const quoteGenerator = (function(){
  let url = "https://andruxnet-random-famous-quotes.p.mashape.com/?cat=famous";
  let apiHeader = new Headers({
    "X-Mashape-Key": "..."
  });
  let init = {
    headers : apiHeader,
  }
  let quoteInfo = {};

  let getQuote = function(){
    fetch(url, init)
      .then(response => {
        if (response.ok){
          return response.json();
        }
        throw new Error("Get request failed");
        }, networkError => console.log("Error: " + networkError))
      .then(jsonResponse => {
        quoteInfo = Promise.resolve({
          quote: jsonResponse.quote,
          author: jsonResponse.author
        }); 
       });
      return quoteInfo;   
  };


  return {
    getQuote : getQuote
  };

})();

//Triggers when user clicks on social media button to share quote.
//Checks to see if quote has been generated from quoteGenerator before
//sharing it, if not, then button does nothing.

const clickHandler = (function(){
  let triggerClicked = function(){
    $("#twitter").on("click", function(e){
      e.preventDefault();
      window.open("https://twitter.com/intent/tweet?text=asdf");
    });

    $("#newQuoteButton").on("click", function(e){
        //ERROR BELOW//
        quoteGenerator.getQuote().then(function(quoteInfo){
        //ERROR ABOVE//
        $("#quoteSection > h2").html("<i class='fa fa-quote-left'></i>" + 
                                     quoteInfo.quote + "<i class='fa fa-quote-right'></i>");
        $("#author").html("<i>- " + quoteInfo.author + "</i>");
      }).catch(function(e){
        console.log(e);
      });
    });
  };
  return {
    handleClicks: triggerClicked
  }
})();

clickHandler.handleClicks();
Justin
  • 165
  • 3
  • 14

2 Answers2

4

Return the data within Promise chain, and as pointed out by @nnnnnn

getQuote() returns quoteInfo. The first time it is called the promise has not yet completed, so quoteInfo is still the empty {} object it was initialised as.

quoteInfo can be omitted from the code

let getQuote = function() {
    return fetch(url, init)
      .then(response => {
        if (response.ok){
          return response.json();
        }
        throw new Error("Get request failed");
        }, networkError => console.log("Error: " + networkError))
      .then(jsonResponse => {
        return {
          quote: jsonResponse.quote,
          author: jsonResponse.author
        }; 
       });  
  };

  $("#newQuoteButton").on("click", function(e){
    //ERROR BELOW//
    quoteGenerator.getQuote().then(function(quoteInfo){
    //ERROR ABOVE//
    $("#quoteSection > h2")
    .html("<i class='fa fa-quote-left'></i>" 
    + quoteInfo.quote + "<i class='fa fa-quote-right'></i>");
    $("#author").html("<i>- " + quoteInfo.author + "</i>");
    }).catch(function(e){
      console.log(e);
    });
  });
guest271314
  • 1
  • 15
  • 104
  • 177
  • Your `quoteGenerator()` function is empty, so `quoteGenerator().getQuote()` will give an error. – nnnnnn Oct 09 '17 at 02:09
  • That fixed it! Thanks! May I ask why my code didn't work. How I see it is that my function returns a promise and then I call then to print out the quote info. – Justin Oct 09 '17 at 02:09
  • @Justin A combination of what nnnnnn pointed out at comment https://stackoverflow.com/questions/46637866/promise-then-doesnt-work-on-first-button-click/46637884#comment80224982_46637866, `fetch()` not being returned, a value not being returned from `.then()` see [Why is value undefined at .then() chained to Promise?](https://stackoverflow.com/questions/44439596/why-is-value-undefined-at-then-chained-to-promise?s=13|27.5334); code outside of `Promise` chain, attempting to cast variable to result of asynchronous procedure – guest271314 Oct 09 '17 at 02:11
  • @nnnnnn The function at first code block is simply to illustrate IIFE pattern not being necessary. – guest271314 Oct 09 '17 at 02:15
  • Oh, so you meant that the outer function would still have the variables and other stuff in it, but then the inner function would be called as `quoteGenerator().getQuote()` rather than with the IIFE and called as `quoteGenerator.getQuote()`? – nnnnnn Oct 09 '17 at 02:18
  • @nnnnnn Yes. The gist of issue at Question is what you pointed out at comment and `getQuote` function. Did not view a need for the IIFE, therefore included code pattern illustration of a function that is not an IIFE which returns expected result when called. Is your view the entire code should be at Answer? – guest271314 Oct 09 '17 at 02:19
  • 1
    No, I wouldn't have included all the code if it didn't change, but I would've included a comment something like `// other code here unchanged` to make it clearer how it fits together. – nnnnnn Oct 09 '17 at 02:21
  • @nnnnnn In all fairness, the IIFE pattern returns same result when `getQuote` is adjusted - and `quoteInfo` is omitted from IIFE. Mention of the IIFE can be be removed from Answer altogether. See updated post – guest271314 Oct 09 '17 at 02:26
  • @nnnnnn, Sorry, but could you guys elaborate on the `getQuote() returns quoteInfo. The first time it is called the promise has not yet completed, so quoteInfo is still the empty {} object it was initialised as.` I'm still new to this so I'm a bit slow at working through the code. Specifically, I don't understand the `first time it is called`, because isn't the first time it's called: `quoteGenerator.getQuote().then...` which means call getQuote() and that returns the quoteInfo object. How would the empty object return before the fetch "then" chain finishes? Thanks – Justin Oct 09 '17 at 02:54
  • @Justin `.then()` callback function is called asynchronously. Also, `fetch()` is not returned from function call – guest271314 Oct 09 '17 at 02:55
  • Thanks for taking the time to make examples for me to study. Am I right to say that for the second example, `getQuote()` is suppose to return a promise, but because our last `.then()` doesn't have a return value, then the promise value is undefined, hence when we do `getQuote().then(data => console.log(data))`, it is essentially logging data of an undefined value, and that's where my problem was? – Justin Oct 09 '17 at 04:16
  • Yes, that is part of the issue. – guest271314 Oct 09 '17 at 04:31
  • And for the other part that you mentioned: `.then()`'s callback function is called asynchronously, I think I understand that but after reading this: `"a pending promise can either be fulfilled with a value, or rejected with a reason (error). When either of these options happens, the associated handlers queued up by a promise's then method are called."`, so what I'm confused about is if `.then()` is dependent on the promise being resolved/rejected, how can it also execute asynchronously? Thanks again – Justin Oct 09 '17 at 05:06
  • `.then()` is called when the `Promise` is resolved or rejected. See [Promises/A+](http://promises-aplus.github.com/promises-spec/), [You're Missing the Point of Promises](https://gist.github.com/domenic/3889970) – guest271314 Oct 09 '17 at 05:15
2

You need to return the fetch call

let getQuote = function(){
    return fetch(url, init)   // <<<<<<< use 'return'
      .then(resp => {
       return resp.ok ? resp.json() : Promise.reject(Error('Get request failed'));
      })
      .catch(networkError => {
       console.error('Error: ' + networkError);
       return Promise.reject(networkError); // <<<< have to call reject here, or throw
      })
      .then(jsonResponse => {
        return {
          quote: jsonResponse.quote,
          author: jsonResponse.author
        }; 
      });
  };

dynamic typing FTW :)

4 tips:

  1. use console.error instead of console.log for errors since this writes to stderr not stdout

  2. use single quotes - they are easier to read IMO

  3. Promise catch blocks are usually a little cleaner than using "err-backs", IMO

  4. Use a real Error object for your errors, not just a message, otherwise will be difficult to track down. You don't need the new keyword, just call Error().

Alexander Mills
  • 90,741
  • 139
  • 482
  • 817
  • 1
    Point 2 is, as you said, a matter of opinion, which ironically you don't follow in point 3... – nnnnnn Oct 09 '17 at 02:20
  • haha true, but English is different than code...your editor is likely to highlight the string for you, whether it's single or double quoted. Save yourself the space. – Alexander Mills Oct 09 '17 at 02:51
  • 1
    The space? I edit code in a mono-spaced font, so single- and double-quotes take up the same amount of space. (Yes, I know English is different from code, a point I've made myself in the past, I was just teasing.) – nnnnnn Oct 09 '17 at 02:54
  • haha yeah, I actually forgot to call promise reject in the catch block, fixed – Alexander Mills Oct 09 '17 at 02:55