amark / gun

An open source cybersecurity protocol for syncing decentralized graph data.
https://gun.eco/docs
Other
17.96k stars 1.16k forks source link

Would not be better if the Todo app tutorial used vanilla JavaScript? #1280

Open qgustavor opened 1 year ago

qgustavor commented 1 year ago

Reasons:

  1. It uses an old jQuery version which does not receive patches anymore.
  2. Despite the "you could use React/Vue/etc" most jQuery usage in the tutorial can be replicated with simple vanilla JavaScript without hurting browser compatibility too much and without making the code too complex
  3. The var li = $('#' + id) pattern could be replaced with document.getElementById(id) eliminating the need of escaping the CSS selector: if an item have id like a,body then the line $(li).text(say) would be equivalent to $('#a,body').text(say) and thus override the body contents.

Proposed code:

<html>
  <body>
    <h1>Todo</h1>

    <form id="sign">
      <input id="alias" placeholder="username">
      <input id="pass" type="password" placeholder="passphrase">
      <input id="in" type="submit" value="sign in">
      <input id="up" type="button" value="sign up">
    </form>

    <ul></ul>

    <form id="said">
        <input id="say">
        <input id="speak" type="submit" value="speak">
    </form>

    <script src="https://cdn.jsdelivr.net/npm/gun/gun.js"></script>
    <script src="https://cdn.jsdelivr.net/npm/gun/sea.js"></script>
    <!-- script src="https://cdn.jsdelivr.net/npm/gun/lib/webrtc.js"></script -->
    <script>
    var gun = Gun(['http://localhost:8765/gun', 'https://gun-manhattan.herokuapp.com/gun']);
    var user = gun.user();
    var $ = document.querySelector.bind(document);

    $('#up').addEventListener('click', function(e){
      user.create($('#alias').value, $('#pass').value);
    });

    $('#sign').addEventListener('submit', function(e){
      e.preventDefault();
      user.auth($('#alias').value, $('#pass').value);
    });

    $('#said').addEventListener('submit', function(e){
      e.preventDefault();
      if(!user.is){ return }
      user.get('said').set($('#say').value);
      $('#say').value = '';
    });

    function UI(say, id){
      var li = document.getElementById(id);
      if (!li) {
        li = document.createElement('li');
        li.id = id;
        $('ul').appendChild(li);
      }
      li.innerText = say;
    };

    gun.on('auth', function(){
      $('#sign').style.display = 'none';
      user.get('said').map().once(UI);
    });
    </script>
  </body>
</html>

In order to avoid making the code too verbose document.querySelector is aliased to $. The other changes needed was replacing .on() with .addEventListener(), .hide() with .style.display = 'none' and using document.createElement in a if (){} instead of short-circuiting the element creation (IMHO this change looks easier to understand than before). If Firefox 44 or older need to be supported replace .innerText with .textContent.

qgustavor commented 1 year ago

If no one is against this change I can contribute those changes to the wiki. Beside the code changes above some wording would need to be changed:

I will wait until Sunday or so in case someone wants to give feedback.

draeder commented 1 year ago

The general idea behind using jQuery is for new developers who don't know how to program at all. Vanilla JS is preferred to me also, but it adds additional lines of code and complexity for beginners.

qgustavor commented 1 year ago

I don't think jQuery is meant to beginners, jQuery helped a lot in the past because cross-browser differences: in the past you had to either use it or call a 12 line function to add a class because Internet Explorer, nowadays just call element.classList.add(). Well, IIRC many of good things of modern JS like classList exist because jQuery.

If you compare the code above with the current one you will see that's those are quite similar and there are quite few additional lines of code, like one for the $ shortcut (which uses .bind() but can be rewritten to use a simple named function to make it more beginner friendly) and some for the element creation code (which I think it's more easier to understand than the original code which relies on short-circuiting which does not look beginner friendly).

That being said, if jQuery is a requirement, at least update it to a still supported version and use jQuery.escapeSelector() to fix the bug in the implementation. The latest jQuery, v3.6.1, supports IE9+ even if it's deprecated.

Edit: adding to the above, if jQuery must continue, since there is a "But CDNs are dangerous!" section, why not pointing to a CDN URL for jQuery that includes versioning like https://cdn.jsdelivr.net/npm/jquery@3.6.1/dist/jquery.min.js? It would allow update this tutorial with the latest jQuery version without requiring to update the gun npm package.

amark commented 1 year ago

@qgustavor nice keeping it lean! I actually have lib/dom.js which is a lightweight jquery like replacement. I'm a vanilla fan too.

Your changes are pretty epic. I was mostly just trying to avoid religion (React vs Vue, etc.) tho jQuery vs Vanilla legit. The only things I'm worried about would be (and htis is very subjective) is (A) don't fix what isn't broken (since my life is hectic enough already. These edits seem safe, but QA? I haven't gotten complaints on the tutorial in years, tho I get complaints plenty of other areas) and (B) does it improve comprehension? If you're confident that A is 90%+ and B is 60%+ then I'd say sure. (Tho if I get any complaints I'll probably revert, just as a warning). I mean, its true, people aren't gonna be doing this coding tutorial on a weird non-evergreen browser, so its probably perfectly safe.

Want to help with SecureRender also?

0awful commented 11 months ago

It seems to me like the best direction would be multiple flavors of the same tutorial in different frameworks. React, Vue, Angular, Vanilla JS, and Jquery. (With a "submit your own!" style prompt and a "report an error" button if you want it to be evergreen) If someone was to learn JS today I'm doubtful they'd start with jQuery. I'd argue they're most likely to learn a framework and JS at the same time.

@amark Is this something you'd be open to contributions for? If so where do I contribute them? I'll gladly do React, Svelte, Angular and Vue to kick it off. I'd also help set up "Submit your own!" and "report an error" as I agree it makes most sense for you to focus on the core value prop of library development vs documentation development.