0

So I created a function (onSuccess) that launches when an AJAX request is successful. The problem is that my function launches when my browser loads and I would only like for the function only to be called when a button with id myButton is clicked. I am locating my button using document.getElementById("myButton"); but when I run console.log(document.getElementById("myButton")); in my browser console I get a Null result.

I've looked around and gotten a few suggestions stating that my element wasn't in the DOM when my script ran. I can understand that but when I moved the location of my script and moved the document.getElementById... my script still doesn't work. I viewed a few suggestions on Stackoverflow but they were mostly JQuery solutions. I'm looking for a pure JavaScript solution. My code is below

JSFiddle Example

            var el = document.getElementById("myButton");
            el.onclick = addUser("username", "email", onSuccess);

            function onSuccess(result){
               alert ('successful');
            }



            // Do not modify this function. Add user service wrapper.
            function addUser(username, email, callback) {
                var xhr = new XMLHttpRequest();
                var response;
                var success = (!!Math.round(Math.random()));

                if (!success){
                    response = JSON.stringify({
                        success: success,
                        error: "Oups, something went wrong!"
                    });
                } else {
                    response = JSON.stringify({
                        success: success,
                        user: {
                            username: username,
                            email: email
                        }
                    });   
                }

                xhr.open("POST", "/echo/json/");
                xhr.onload = function () {
                        if (xhr.status === 200) {
                            callback(JSON.parse(xhr.responseText));
                    }
                }
                xhr.send("json=" + response);
            };
Spanky
  • 699
  • 2
  • 11
  • 38

3 Answers3

3

You've gone a little wrong here, but I think I know what you need to do.

You want to pass a function reference to the onclick attribute, but instead you're executing the function and setting onclick to be whatever that function returns.

This is why your function is running when the browser loads. You need to pass the reference; not execute it and pass the output of the function.

So how do you solve it?

There is a cool practice in functional programming called Currying Functions. It's very useful in situations like this, when you need to pass in some parameters but you want to delay execution. In this case, you could do something like this:

function buildAddUser(username, email, callback) { 
    return function() {
        addUser(username, email, callback)
    }
}

Then, you set your button to be equal to the output of buildAddUser..

el.onclick = buildAddUser("username", "email", onSuccess);

This will set the onclick attribute to be equal to a function that invokes the addUser function.

Why not just use an anonymous function?

I prefer to wrap these things in named functions, to make your code easier to read. Anonymous functions flying all over the place can make things confusing.

Extra Reading

Functional Programming in Javascript is one of the most exciting new developments and offers a wide array of tools to help solve problems just like this. You should have a read through some of the material available

christopher
  • 26,815
  • 5
  • 55
  • 89
  • This is great input! Will definitly read up on it. I figure that I should replace `el.onclick = buildAddUser("username", "email", onSuccess);` with `el.onclick = buildAddUser("username", "email", callback);` since you are using that parameter in your example. For some reason it's not working for me. I'm getting a " callback is not defined" error in my Fiddle example – Spanky Jul 06 '17 at 20:49
  • 1
    You can use 'onSuccess' ! The name of the arguments isn't important; the crucial part here is the idea of using one function to hold the parameters and defer execution to a point that you want. – christopher Jul 07 '17 at 03:02
2

The thing that is going wrong is that you are calling addUser when you try to set the onClick listener. Try this instead.

var el = document.getElementById("myButton");
el.onclick = function() {
  var username = document.getElementById("username").value;
  var email = document.getElementById("email").value
  addUser(username, email, onSuccess)
}

function onSuccess(result){
   alert ('successful');
}



// Do not modify this function. Add user service wrapper.
function addUser(username, email, callback) {
    var xhr = new XMLHttpRequest();
    var response;
    var success = (!!Math.round(Math.random()));
    
    if (!success){
        response = JSON.stringify({
            success: success,
            error: "Oups, something went wrong!"
        });
    } else {
        response = JSON.stringify({
            success: success,
            user: {
                username: username,
                email: email
            }
        });   
    }
    
    xhr.open("POST", "/echo/json/");
    xhr.onload = function () {
      if (xhr.status === 200) {
          callback(JSON.parse(xhr.responseText));
        }
    }
    xhr.send("json=" + response);
};
<h2>Add a User:</h2>
<input id="username" type="text" name="username" placeholder="name">
<input id="email" type="email" name="email" placeholder="email">
<button id="myButton">add user</button>
<h2>Users:</h2>
<ul id="users"></ul>
frithjof
  • 345
  • 3
  • 8
  • This is another good way around the problem! Nice thinking. +1 – christopher Jul 06 '17 at 20:38
  • @Jimmy can you tell me what error you are getting? notice I also added id in the Html so I can get the values. – frithjof Jul 06 '17 at 22:17
  • @Jimmy The snippet doesn't respond as intended on SO, I edited your jsFiddle so now it should work [JsFiddle](https://jsfiddle.net/Frithjof/jetmh6me/1/) – frithjof Jul 06 '17 at 22:44
0

Create anonymous function call which wraps the function

el.onclick = function() { addUser("username", "email", onSuccess); };

For more detailed explanation you can refer here

Kukic Vladimir
  • 1,010
  • 4
  • 15
  • 22