Building a basic framework

I am trying to build a very basic JS framework for my own uses, ultimately collating a lot of pieces of JavaScript I have created over the years. But I would not call myself a JavaScript expert.

What I have written below works well enough but I really would like to know if I am making any fundamental errors in the way this is set up, rather than re-writing a lot of code further down the line.

This basic start point is mirroring the way jQuery can be written to select an element and chain an event to that object, but using all the querySelectorAll instead of the older methods.

If anyone can find basic or advanced flaws in what I've done that would be most useful.

///DO NOT COPY OR USE FOR COMMERCIAL PURPOSES
function Framework(sel) {

    function myFramework(sel) {

        ///Add selections to array 

        this.sel = sel;

        if (typeof this.sel !== "undefined") {

            if (typeof this.sel !== 'string') {
                this.selArray = new Array(this.sel);
            } else {
                this.what = this.sel.charAt(0);

                if (this.what == ".") {
                    this.selArray = document.querySelectorAll(this.sel);
                } else if (this.what == "[") {
                    this.selArray = document.querySelectorAll(this.sel);
                } else if (this.what == "#") {
                    this.sel = this.sel.substr(1);
                    this.selArray = new Array(document.getElementById(this.sel));
                }
            }
        }

        myFramework.prototype.ready = function (func) {

            document.onreadystatechange = function () {
                var state = document.readyState
                if (state == 'interactive') {
                    //  init();
                } else if (state == 'complete') {
                    func();
                }
            }
        }

        myFramework.prototype.click = function (func) {
            for (i = 0; i 

    

What you've got is not a framework. It is a library.

I can see you are basically creating a jQuery-like wrapper for selecting DOM nodes and event handling. Nothing terribly wrong with the main idea.

Why have a function called Framework which has a function inside of it? Every call to Framework(...) causes myFramework to be recreated. Instead, just use a regular prototype-based "class" in JavaScript:

function Framework(sel) {
    if (!(this instanceof Framework)) {
        // return a new Framework object if called without the "new" keyword
        return new Framework(sel);
    }

    this.sel = sel;

    if (this.sel) {
        if (typeof this.sel !== 'string') {
            this.selArray = new Array(this.sel);
        } else {
            this.what = this.sel.charAt(0);

            if (this.what == ".") {
                this.selArray = document.querySelectorAll(this.sel);
            } else if (this.what == "[") {
                this.selArray = document.querySelectorAll(this.sel);
            } else if (this.what == "#") {
                this.sel = this.sel.substr(1);
                this.selArray = new Array(document.getElementById(this.sel));
            }
        }
    }
}

Framework.prototype = {
    constructor: Framework,

    ready: function(func) {
        // ...
    },

    click: function(func) {
        // ...
    }
};

The code that inspects the selector and calls querySelectorAll or getElementById is completely redundant and unneeded. If this.sel is a string, just blindly pass it to document.querySelectorAll. You've gained nothing by inspecting the selector. The speed difference between getElementById("foo") and querySelectorAll("#foo") will be negligible at worst, so why complicate your code?

if (this.sel) {
    this.selArray = typeof this.sel === "string"
        ? document.querySelectorAll(this.sel)
        : [this.sel];
}

Optimizing The Attaching Of Events

In your code, you loop over the array of elements, and on each iteration you test for attachEvent or addEventListener. Browsers won't magically change their native DOM API, so testing for these methods repeatedly when the result will be the same is extra work for the browser. Instead, create a method that does the event binding, which we will call bind.

Instead of testing for the "attachEvent" method on each iteration of the loop in click, test it once:

Framework.prototype = {
    constructor: Framework,

    ready: function(func) {
        // ...
    },

    click: function(func) {
        for (var i = 0; i < this.selArray.length; i++) {
            this.bind(this.selArray[i], "click", func);
        }

        return this;
    },

    bind: !!document.addEventListener
        ? function(element, event, func) {
            element.addEventListener(event, func, false);
            return this;
        }
        : function(element, event, func) {
            element.attachEvent(event, "on" + func);
            return this;
        }
};

The bind method is set on the prototype and makes use of a ternary operator to assign a function to the bind property that supports the current browser's native API instead of checking for support.

// The Test: Does the browser have a document.addEventListener method?
    bind: !!document.addEventListener

     // Yes: The browser does. Use this function to bind events using standard API
        ? function(element, event, func) {
            element.addEventListener(event, func, false);
            return this;
        }

     // No: The browse does not. Use Internet Explorer's API
        : function(element, event, func) {
            element.attachEvent(event, "on" + func);
            return this;
        }

The decision to use attachEvent or addEventListener is made once at page load. The Framework.prototype.bind method will directly use the supported API for each browser rather than test for it.

More information on ternary operators: Conditional (ternary) Operator

Implicitly Declared Global Variables

Don't forget that variable assignments without var before the variable name create implicitly declared global variables. The for loop in the click method does this with the i variable, and your original Framework method mysteriously declares a global variable newTask.

StackExchange Code Review Q#67709, answer score: 3