Docs: Don't show a useless use of var
Closed this issue · 5 comments
The docs say
<script type="text/javascript" src="https://pym.nprapps.org/pym.v1.min.js"></script>
<script>
var pymChild = pym.Child({ renderCallback: myFunc });
</script>This use of var is useless. Y'all probably know all about this because you know JavaScript well, but I've seen people blindly copy and paste this code without realizing that var pymChild sets a variable on window. I suggest either explicitly setting it to window.pymChild = (so it's more clear) or just pym.Child({ renderCallback: myFunc });.
I don't agree that it's useless. I don't want to sidetrack the docs having to explain how top-level variable declarations work in JavaScript, or encouraging people to assign to the window object unthinkingly if they're already copy/pasting code.
We could revise it to let, which does not add properties to the global object. But since Pym is generally on its way out in favor of Sidechain anyway, I'm not particularly driven to spend a lot of time fixing all the old-school flavor of the docs--particularly since that would mean I should probably fix the old-school flavor of the code, and I have no intention of making changes to Pym itself if I can help it.
It is useless in the literal sense that these three snippets are absolutely identical:
A)
<script>
var pymChild = pym.Child({ renderCallback: myFunc });
</script>B)
<script>
pymChild = pym.Child({ renderCallback: myFunc });
</script>C)
<script>
window.pymChild = pym.Child({ renderCallback: myFunc });
</script>I think C is the most clear to readers, but B is also fine. A is bad because it is misleading and makes pymChild look like a local variable.
encouraging people to assign to the window object unthinkingly if they're already copy/pasting code
The docs are already doing that though?
No, it encourages them to create a local variable. That has the side effect, at a top-level scope, of assigning to the global object. It may seem like a distinction without a difference, but I think showing people who are relatively inexperienced at JS (which is the kind of person you're talking about here) that they should define values by either leaving off the var keyword or explicitly assign to the global object is a bad practice, especially if they take that lesson and start using Pym inside of an init function or something similar.
Yes, in this particular case of a bare script tag on the window, using var has side effects. But that's not a bug in Pym, it's a bug in JS. I think the alternatives are worse. And unless you're worried that there's going to be a window.pymChild value that would be shadowed or overwritten by the assignment, I don't understand what the harm is. In the context of a normal page (particularly an iframed child page--and thus sandboxed/isolated JS context), this is a perfectly reasonable thing to do.
I don't think we're going to agree, so I'll let this be my last comment, but I think that many casual users of JavaScript have no idea what var does, and seeing a code sample which creates a global using var just adds to their confusion. For most use of Pym, there's nothing wrong with window.pymChild, but it's better to be explicit rather than implicit when using it.