3
\$\begingroup\$

Long story short: I wanted some of the time-saving functionality of jQuery without the bulk of the entire library. So, I wrote a lightweight shorthand library inspired by jQuery I'm calling Mozart JS, and I'd like to see how I can further enhance it without major impact on load times.

Thoughts? Full library on GitHub.


Mozart JS

Element Retrieval

Fairly straightforward.

function $get(e, t) {
    if (void 0 === t) t = document;
    if (e.startsWith('#')) return t.getElementById(e.substring(1));
    if (e.startsWith('.')) return t.getElementsByClassName(e.substring(1));
    if (e.startsWith('-')) return t.getElementsByName(e.substring(1));
    return t.getElementsByTagName(e);
}

Example:

var foo = $get('#bar'); //returns element with the Id of "bar"
var zed = $get('.cop'); //returns collection with the classname "cop" 

Element Creation

Slightly more complicated; allows the assignment of common properties and all attributes.

function $make(t, p) {
    var o = document.createElement(t);
    if (p.style) o.style.cssText = p.style;
    if (p.class) o.className = p.class;
    if (p.text) o.textContent = p.text;
    if (p.value) o.value = p.value;
    Object.keys(p).forEach((e) => {
        if(e.startsWith("@")) {
            o.setAttribute(e.substring(1), p[e]);            
        }        
    });
    return o;
}

Example:

var h = $make('a', {
    'class': 'myClass',
    'text': 'Google',
    '@id': 'googleLink',
    '@href': 'https://google.com',
    '@target': '_blank'
});

//Equivalent to:

var h = document.createElement('a');
h.className = 'myClass';
h.textContent = 'Google';
h.setAttribute('id','googleLink');
h.setAttribute('href', 'https://google.com');
h.setAttribute('target', '_blank');

Element Sibling Insertion

Designed to mimic most of jQuery's insertBefore() method. Equivalent function exists for insertAfter(); omitted here in order to save space.

function $addBefore(e, t) {
    if (typeof t === 'string' || t instanceof String) {
        if (t.startsWith('#')) {            
            $get(t).parentNode.insertBefore(e, $get(t));
        } else {
            var arr = $get(t);
            for (var i in arr) {
                var f = e.cloneNode(true);
                arr[i].parentNode.insertBefore(f, arr[i]);
            } 
        }
    } else {
        t.parentNode.insertBefore(e, t);
    }        
}

Example:

var foo = $make('div', {'text': 'This is a test'});
$addBefore(foo, '.myClass'); //adds an instance of foo before each myClass

Element Removal

Removes an element. Still uses basic jQuery syntax.

function $remove(e) {    
    if (typeof e === 'string' || e instanceof String) {
        $remove($get(e));
    } else if (e.nodeType === 1 || e.nodeType === 3) {
        e.parentNode.removeChild(e);
    } else {
        for (var i in e) {
            e[i].parentNode.removeChild(e[i]);
        }
    }
}

Example:

$remove('#myId');
$remove('.myClass');

//Equivalent to:

var rem = document.getElementById('myId');
rem.parentNode.remove(rem);
for (var i in document.getElementsByClassName(myClass)) {
    var elm = document.getElementsByClassName(myClass)[i];
    elm.parentNode.remove(elm);
}
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Good job on your first question. \$\endgroup\$
    – SirPython
    Jan 25, 2016 at 23:34
  • \$\begingroup\$ nice project, I already have starred it :). I'll keep watching it. \$\endgroup\$ Jan 31, 2016 at 0:01

2 Answers 2

2
\$\begingroup\$

I wanted some of the time-saving functionality of jQuery without the bulk of the entire library.

If you don't look back at older browsers, use version 2.x of jQuery. It's smaller than the 1.x as it has less cross-browser compatibility code (See How 2.0 Changed section). You can also custom-build jQuery to your own spec to include what you need. An even smaller yet jQuery-compatible library is Zepto, just ~9k compressed.

Element Retrieval

Pretty straightforward. However, there is querySelectorAll which pretty much acts like jQuery's $. A singular version of it is querySelector. It's native, and has been there for quite a while.

One neat thing about jQuery and jQuery-like libraries is that they encapsulate results, single or multiple, into a set. Methods operate by looping through and applying to each item in the set. No items, no loop, thus no throw.

$('#foo').children().eq().next().remove();

The following example will not throw an error even when there's no #foo in the page.

Element Creation

var h = $make('a', {
    'class': 'myClass',
    'text': 'Google',
    '@id': 'googleLink',
    '@href': 'https://google.com',
    '@target': '_blank'
});

Not entirely sure what's the difference between class and the other attributes when it's also an attribute. What should be different is text since it doesn't actually describe the element. Also, text nodes can be created separately via createTextNode.

Element Sibling Insertion

if (t.startsWith('#')) {            
  $get(t).parentNode.insertBefore(e, $get(t));
} else {
  var arr = $get(t);
  for (var i in arr) {
    var f = e.cloneNode(true);
    arr[i].parentNode.insertBefore(f, arr[i]);
  } 
}

This is one case where your code will start to become unmaintainable. This function is making a special case for singular results. If you go with the set-like behavior of jQuery, singular results are still a set. Your code will always assume a set and be more consistent in operation.

Element Removal

function $remove(e) {    
    if (typeof e === 'string' || e instanceof String) {
        $remove($get(e));
    } else if (e.nodeType === 1 || e.nodeType === 3) {
        e.parentNode.removeChild(e);
    } else {
        for (var i in e) {
            e[i].parentNode.removeChild(e[i]);
        }
    }
}

Again, your code is doing special cases for different things. This could have been easier if you operated with a set of things.

function $remove(input){

  // Building the set
  var set = inputIsAString ? lookForElements(input)
          : inputIsAnElement ? createASet(input)
          : inputIsAlreadyASet ? input
          : [];

  // The actual operation
  set.forEach(function(){
    // element removal code
  });
}
\$\endgroup\$
1
\$\begingroup\$

Element retrieval

Versatility

Your $get is good; it allows for easy element finding. However, it isn't very flexible and would not be very useful in a larger application.

What if there is an element that can only be selected with div.foo.bar? This function would not work for this as it can only one type of selector and one selector at a time.

Now, to make this function more versatile, there are one of two things you can do:

  • You can do it yourself. This would require a bit more work, but you split up the query for each symbol into an array. Then, you simply have to iterate through the array, finding the elements matching that specific selector and combing the results into an array with the other selectors, removing the elements that are not common across selectors.

For example, if I passed this: div.foo.bar, the function would split it into div, .foo, and .bar. Then, it would go through finding all the elements for each selector and comparing them with a final array.

  • Just use document.querySelectorAll

This method does just what your function is doing, only it allows for inputs such as div.foo.bar, returning an array of all matches. This would probably be the best option, unless you wanted to try and implement it yourself (like above).


Different types

This function's returning get's really confusing, and makes your code very hard to use (as evident of how you are using it in $addBefore, which I'll discuss later).

Your function can return one of the following:

  • A single DOM element
  • An array of DOM elements

The single DOM element is returned from getElementById. This can make working with the return of this function especially difficult if you don't know what the input is like.

An easy fix:

if (e.startsWith('#')) return [t.getElementById(e.substring(1))];

See it? All I did was add []s around the method call. This sticks the output in an array and returns the array.

This will greatly help your library as I will show later.


Element Creation

Strange @

Why are you forcing properties that are not

  • style
  • class
  • text
  • value

to have a @ at the beginning? This just makes things more difficult because (1) this function has to worry about substrings when building the element, and (2) if the code using this library is copying some elements from another element, they are forced to manually add the @ to the beginning of some properties (which makes it harder to loop through properties and copy them, and also slows down the code with unnecessary string concatenation).

It would be a lot easier to use this part of the library (and the code would be slightly faster) if you didn't force a @ to be there, and instead just let the property name sit as it.


For in loop

This line:

Object.keys(p).forEach((e) => {

is equivalent to:

for(var e in p) {

The line I wrote should be used because it is common JavaScript practice. What you wrote there looks like you're just trying to fake a for-in loop, and looks quite ugly.

Also, I recommend using more descriptive names than e and p (and this applies to your argument names, too).


Element Sibling Insertion

ID's versus classes/names/tags

In the earlier part of my answer, I stated a big problem with your $get function returning different typed values. The problem is evident in this function:

if (t.startsWith('#')) {            
    ...
} else {
    ...
}

See that? You have to write two entirely separate clauses just to handle ID's. That's pretty ugly.

However, if you follow that advice of forcing the $get function to return an array no matter what, then you can remove this conditional all together and just keep the second clause, which handles arrays.


Properly iterating through an array

This is how you are iterating through an array:

for (var i in arr) {

No. That is very bad JavaScript practice. The better way to do it would be to write it like a normal for loop:

for(var i = 0, len = arr.length; i < len; i++) {

}

Seems to me that you need a little review on when to use which types of loops in JavaScript.


Too much $get(t)

Although it may not seem like it, your function $get can be pretty intensive considering it is making calls to methods that have to traverse the entire DOM in search for some elements.

While this is not bad (and is probably as fast as you are going to get it), you want to minimize your usage of this function by storing the return in a variable.

Now, following the above advice where you remove the conditional, that will no longer be a problem in this function. However, if you have something like this:

$get(t).doStuff();
otherThing.doStuffTo($get(t), $get(t).parent);

where you are making a lot of calls to $get(t), it would be best to store the return of $get(t) in a variable and then to just access that variable.


Versatility

Right now, this function will only copy elements to the space before all the elements in an array that would be returned from an array-inducing selector (such as .) if the second argument is a string.

But what if I, the code using your library, has already gotten the elements with $get? Now I can't use your function because it only accepts single elements in t when it is not of type string.

To make your code more versatile, you should expect that t is an array if not a string, and then to loop through the array appending the element like you do in the part with the string, like this:

} else {
    for (var i = 0, len = t.length; i < len; i++) {
        var f = e.cloneNode(true);
        t[i].parentNode.insertBefore(f, arr[i]);
    } 
}  

Then, you can simplify your code even further and do what you are doing in $remove: calling the function again, but with the return of $get. Here is the whole function now:

function $addBefore(e, t) {
    if (typeof t === 'string' || t instanceof String) {
        $addBefore(e, $get(t));
    } else {
        for (var i = 0, len = t.length; i < len; i++) {
            var f = e.cloneNode(true);
            t[i].parentNode.insertBefore(f, arr[i]);
        } 
    }       
}

Element Removal

Nothing here that I haven't already talked about.


General

Argument names

You may have seen this in other libraries, but it's not good practice: your argument names should be long enough so that their purpose is understandable (e and t to not explain the variable's purpose).

Type checking

You are really relying on the user here to pass the correct types. Don't. Remember:

The customer is almost never right.

You should spit out an error (like TypeError) immediately at the user and turn your back to them.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the feedback! I'm honestly fairly new to javascript so I had no idea querySelectorAll even existed. Can you elaborate on why (for var i in...) loops are bad? I thought it was just shorthand. \$\endgroup\$
    – Cliff
    Jan 26, 2016 at 3:26
  • \$\begingroup\$ @Cliff Welcome to JavaScript! for in loops aren't bad; they are bad practice if used to iterate over an array. They should be used to iterate over object literals, such as {foo: 'bar'} \$\endgroup\$
    – SirPython
    Jan 27, 2016 at 0:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.