Revision cdae90c5-52c6-48d8-988f-eeea33c33d19 - Code Review Stack Exchange
My goal with this review is to receive observations and suggestions for improving the efficiency of writing the front end of a web application with this basic framework I've fleshed out. Not interested in performance efficiency, but specifically code patterns and design of the resulting applications.
Designing web apps I found two things I didn't like about ReactJS (at least for my typical project scale):
- Had to write more code than I wanted to accomplish basic things
- Was tedious to transport information through the app, you had to essentially pass a wire through the components to get information from point A to point B, which made the design feel tightly coupled and difficult to re-structure afterwards
I also sort of felt this way about other JS app frameworks I tried. So I wrote two fairly simple classes that work together to create a development pattern that I preferred. My goal was for this to let me:
1. Focus all of the process of building each individual component of an
app into modular JS without having to care much about how each component is connected to the
outside application.
2. Not have to go between multiple files or languages to edit JavaScript, HTML and CSS to build or maintain any one UI feature.
Loose coupling, separation of concerns, pure JS workflow, predictable project structure, easy data flow. That was the goal. I don't know if I've achieved that goal, but I've tried, and hopefully after I finish developing my current app with this design and receive feedback here, I'll learn what my errors are and how to polish this into something that accomplishes my goals as efficiently as I'd like.
<sub>*Note: JSX sort of does some of `#2`, but having the two languages in one file felt a bit odd to me, I wanted my files to be a uniform language rather than JSX woven through it like with React. Sorry to any React fans I'm offending out there :P, just a matter of preference I guess.*</sub>
---
Components:
-----------
1. **Publisher.js** - a simple message-passing class to implement the [Pub Sub pattern][1] because I wanted to be able to send namespace-separated events from any location in the app and read them anywhere else, like: `publisher.emit("header/select-nav/home", this)` and `publisher.on("header/select-nav/" + name, data => {})`. Additionally, I support a third `bool` argument to support sending and listening for events over an optionally passed in Socket.io socket, like `let publisher = new Publisher(io())`, so I could handle local and remote events in the same way.
Usage:
let publisher = new Publisher(io()) // or let publisher = new Publisher()
publisher.on("namespace1/subnamespace2/event-name", data => {}, false)
// third arg set to true tells the event handler to listen for Socket.io events
2. **Element.js** - a wrapper for HTML elements that facilitates the entirety of the app's HTML generation and logic, so the logic associated with each visible component of the app is tightly coupled to it, while all the components individually remain loosely coupled to each-other. I also plan to maybe add support for generating CSS classes locally within each component too.
Usage:
new Element("div", { // create a div
id: "header", // set the id
traits: { publisher }, // assign data that will be inherited by any children
data: { name: "primary" }, // assign data to this element only
text: "My header text", // set text
innerHTML: false, // set innerHTML
classes: [], // assign classes
attributes: { // assign attributes
"contenteditable": "true"
},
styles: {}, // assign styles
actions: { // create actions
show: self => {
self.clearStyle("opacity") // clear styles
self.addClass("visible") // add classes
},
hide: self => {
self.removeClass("visible") // remove them
self.style("opacity", 0) // or set styles
}
},
callback: self => { // trigger as soon as the element is created
self.append(new Element("div", {
id: "important-icon",
classes: ["hidden", "header-icon"],
actions: {
select: () => {
self.addClass("visible")
self.removeClass("hidden")
self.updateText("Selected") // update text
}
},
ready: self => {
self.on("mouseover", evt => { // handle DOM events
self.addClass("highlight")
})
}
}))
},
ready: self => { // trigger after the element appends children
self.traits.publisher.on("events/header/" + self.data.name, data => {
self.select("#important-icon").actions.select();
// you could of course apply this listener to the icon itself, but the
}) // select feature is convenient in some cases
}
}).appendTo(document.body)
3. **Controller.js** - validation of input during data flow grows more and more important the larger an application becomes. So it should be a choice of course, whether you want to use it, and I made it available and supported for validating the data flow both within the Element *and* in the Publisher. I didn't code in publisher support yet but it'll work the same at `Element`, with `publisher.use(controller)`. But I also wanted a pass to pass a blueprint input to a set of elements requiring the same properties, and it makes sense for a Controller to be able to override the current input passing through it for ease of testing / debugging, so I added an `insert` method to it, which (as you'll see in the code) can and should be used for templating Element properties.
Usage:
let page = new Controller({
data: data => { // pass a function to validate data however you want
if (!data.name) return false
else return true
},
traits: true, // pass true to simply ensure a setting is passed
actions: "object", // pass a string to test against typeof
}).insert({ // and insert specific default data
traits: {
publisher
},
actions: {
select: self => {
let target = "header/select-nav/" + self.data.name.toLowerCase()
self.traits.publisher.emit(target, this)
self.addClass("visible")
}
},
ready: self => {
self.traits.publisher.emit("header/add-nav", self)
}
});
Element.js:
-----------
import Controller from "/js/classes/controller.js"
function isCyclic(obj) {
var seenObjects = [];
function detect(obj) {
if (obj && typeof obj === 'object') {
if (seenObjects.indexOf(obj) !== -1) {
return true;
}
seenObjects.push(obj);
for (var key in obj) {
if (obj.hasOwnProperty(key) && detect(obj[key])) {
//console.log(obj, 'cycle at ' + key);
return true;
}
}
}
return false;
}
return detect(obj);
}
function isObject(item) {
return item && typeof item === 'object' && !Array.isArray(item);
}
function isIterable(item) {
let type = false;
if (isObject(item)) type = 'obj';
else if (Array.isArray(item)) type = 'arr';
return type;
}
function mergeDeeper(source, target) {
let allProps = [];
let sourceProps;
let type;
let targetProps;
if (isObject(source)) {
sourceProps = Object.keys(source);
type = 'obj';
} else if (Array.isArray(source)) {
sourceProps = source;
type = 'arr';
} else {
return source;
}
if (isObject(target)) {
targetProps = Object.keys(target);
} else if (Array.isArray(target)) {
targetProps = target;
} else {
throw "target missing"
}
sourceProps.forEach(prop => {
allProps.push(prop);
});
targetProps.forEach(prop => {
allProps.push(prop);
});
allProps = [...new Set(allProps)];
let merged
if (type == 'obj') {
merged = {};
} else if (type == 'arr') {
merged = [];
}
allProps.forEach(prop => {
if (type == "obj") {
if (source[prop]) {
if (isIterable(source[prop])) {
if (target[prop] !== undefined) merged[prop] = mergeDeeper(source[prop], target[prop])
else merged[prop] = source[prop]
} else merged[prop] = source[prop]
} else {
if (isIterable(target[prop])) {
if (source[prop] !== undefined) merged[prop] = mergeDeeper(target[prop], source[prop])
else merged[prop] = target[prop]
} else merged[prop] = target[prop]
}
} else {
let iterable = isIterable(prop);
if (iterable) {
let filler
if (iterable == "obj") filler = {};
else if (iterable == "arr") filler = [];
merged.push(mergeDeeper(prop, filler))
} else {
merged.push(prop)
}
}
})
return merged;
}
const collectChildSelectors = (elementWrapper, selectors) => {
elementWrapper.children.forEach(childWrapper => {
if (childWrapper.element.id) {
selectors[childWrapper.element.id] = childWrapper
}
if (childWrapper.selector) {
selectors[childWrapper.selector] = childWrapper
}
collectChildSelectors(childWrapper, selectors)
})
}
const applySettings = function(newSettings) {
if (!newSettings) throw "bad settings"
let settings = mergeDeeper(newSettings, {
text: false,
innerHTML: false,
classes: [],
actions: {},
data: {},
attributes: {},
styles: {},
traits: {},
id: false,
callback: false,
ready: false,
});
if (settings.id) {
this.element.id = settings.id
this.selector = settings.id
}
if (settings.text) this.element.textContent = settings.text
if (settings.innerHTML) this.element.innerHTML = settings.innerHTML
if (settings.selector) {
this.selector = settings.selector
this.selectors[settings.selector] = this;
}
settings.classes.forEach(className => this.element.classList.add(className))
Object.keys(settings.attributes).forEach(attributeName => this.element.setAttribute(attributeName, settings.attributes[attributeName]))
Object.keys(settings.styles).forEach(styleName => this.element.style[styleName] = settings.styles[styleName])
Object.keys(settings.actions).forEach(actionName => this.actions[actionName] = () => settings.actions[actionName](this))
Object.keys(settings.data).forEach(propertyName => this.data[propertyName] = settings.data[propertyName])
Object.keys(settings.traits).forEach(propertyName => this.traits[propertyName] = settings.traits[propertyName])
if (settings.ready) this.ready = settings.ready
if (settings.callback) settings.callback(this);
}
export default class {
constructor(tag, settings) {
this.children = [];
this.data = {}
this.actions = {}
this.traits = {}
this.selectors = {}
this.element = document.createElement(tag)
applySettings.apply(this, [settings])
}
use(arg1, arg2) {
if (arg1 instanceof Controller) {
let controller = arg1;
let settings = arg2;
let mergedSettings = mergeDeeper(settings, controller.insertions);
controller.test(mergedSettings);
applySettings.apply(this, [mergedSettings])
} else if (arguments.length === 1) {
let settings = arg1;
applySettings.apply(this, [settings])
} else {
throw "bad settings passed to Element"
}
return this;
}
addEventListener(event, func) {
this.element.addEventListener(event, func)
}
delete() {
this.parent.removeChild(this.element)
}
style(styleName, value) {
this.element.style[styleName] = value
}
clearStyle(styleName) {
this.element.style[styleName] = ""
}
updateText(text) {
this.element.textContent = text
}
updateAttribute(attributeName, attributeContent) {
this.element.setAttribute(attributeName, attributeContent)
}
addClass(className) {
this.element.classList.add(className)
}
removeClass(className) {
this.element.classList.remove(className)
}
on(evt, func) {
this.element.addEventListener(evt, func)
}
select(id) {
let parts = id.split("#")
let selector = parts[parts.length - 1];
if (!this.selectors[selector]) debugger; //throw "bad selector " + selector
return this.selectors[selector]
}
appendTo(elementWrapper) {
let element
if (elementWrapper.nodeName) element = elementWrapper
else {
element = elementWrapper.element
this.parent = element
collectChildSelectors(this, elementWrapper.selectors)
Object.keys(elementWrapper.traits).forEach(propertyName => this.traits[propertyName] = elementWrapper.traits[propertyName])
}
if (this.ready) this.ready(this)
element.appendChild(this.element)
return this
}
append(elementWrapper) {
let element
let wrapped = false
if (elementWrapper.nodeName) element = elementWrapper
else {
wrapped = true
element = elementWrapper.element
element.parent = this
if (element.id) this.selectors[element.id] = elementWrapper
if (elementWrapper.selector) this.selectors[elementWrapper.selector] = elementWrapper
this.children.push(elementWrapper)
collectChildSelectors(elementWrapper, this.selectors)
Object.keys(this.traits).forEach(propertyName => elementWrapper.traits[propertyName] = this.traits[propertyName])
}
if (elementWrapper.ready) elementWrapper.ready(elementWrapper)
this.element.appendChild(element)
if (wrapped) return elementWrapper
}
}
Controller.js:
--------------
export default class {
constructor(settings) {
this.tests = {};
Object.keys(settings).forEach(key => {
let val = settings[key];
if (typeof val == "boolean") {
this.tests[key] = input => {
return input !== undefined
}
} else if (typeof val == "string") {
this.tests[key] = input => {
return typeof input === val
}
} else if (typeof val == "function") {
this.tests[key] = val;
}
})
}
test(obj) {
Object.keys(obj).forEach(key => {
if (!this.tests[key] || !this.tests[key](obj[key])) {
console.log("Controller test failed");
debugger;
}
});
}
insert(insertion) {
this.insertions = insertion;
return this;
}
}
Publisher.js
------------
export default class {
constructor(socket) {
if (socket) this.socket = socket;
this.events = {};
}
on(command, func, socket = false) {
if (!this.events[command]) this.events[command] = [];
this.events[command].push(func);
if (socket && this.socket) socket.on(command, func);
}
emit(command, data = {}, socket = false) {
if (this.events[command]) {
this.events[command].forEach(func => func(data));
}
if (socket && this.socket) socket.emit(command, data);
}
}
Implimentation
--------------
**`app.js`:**
import Publisher from "/js/classes/publisher.js"
import Controller from "/js/classes/controller.js"
let publisher = new Publisher(io())
import Header from "/js/classes/header/header.js"
import Home from "/js/classes/pages/home/home.js"
import News from "/js/classes/pages/news/news.js"
import Leaderboard from "/js/classes/pages/leaderboard/leaderboard.js"
import Account from "/js/classes/pages/account/account.js"
import Builder from "/js/classes/pages/builder/builder.js"
let header = new Header(publisher)
let page = new Controller({
data: true, // () => { } // validate the value however you choose
traits: true, // It's good to have this capability for debugging
actions: true, // or for if your boss wants all your data interfaces
ready: true // validated because he read it in a hip dev blog
}).insert({ // <- But insertion is the feature you'll be using
traits: { // more often to test input data, debug, and like with
publisher // this case, apply a single input object to multiple
}, // Elements
actions: {
select: self => {
let target = "header/select-nav/" + self.data.name.toLowerCase()
self.traits.publisher.emit(target, this)
self.addClass("visible")
}
},
ready: self => {
self.traits.publisher.emit("header/add-nav", self)
}
});
new Home().use(page, {
data: {
name: "Home",
iconPath: "/assets/home/home-1.png",
cornerPath: "/assets/corners/corner-1.png",
}
}).appendTo(document.body)
new News().use(page, {
data: {
name: "News",
iconPath: "/assets/news/news-1.png",
cornerPath: "/assets/corners/corner-5.png"
}
}).appendTo(document.body)
new Leaderboard().use(page, {
data: {
name: "Leaderboard",
iconPath: "/assets/leaderboard/leaderboard-1.png",
cornerPath: "/assets/corners/corner-3.png",
}
}).appendTo(document.body)
new Account().use(page, {
data: {
name: "Account",
iconPath: "./assets/profile/profile-1.png",
cornerPath: "/assets/corners/corner-4.png",
}
}).appendTo(document.body)
new Builder().use(page, {
data: {
name: "Builder",
iconPath: "./assets/builder/builder-1.png",
cornerPath: "/assets/corners/corner-2.png",
}
}).appendTo(document.body).actions.select()
**`/js/classes/pages/builder/builder.js`:**
import Element from "/js/classes/element.js"
import NavBar from "/js/classes/pages/builder/nav-bar.js"
export default class {
constructor() {
return new Element("div", {
id: "builder",
classes: ["page"],
actions: {
select: self => {
let target = "header/select-nav/" + self.data.name.toLowerCase()
self.traits.publisher.emit(target, this)
self.addClass("visible")
}
},
ready: self => {
self.traits.publisher.emit("header/add-nav", self)
self.actions.select()
},
callback: self => {
self.append(new NavBar());
// add more elements
}
})
}
}
**`/js/classes/pages/header/header.js`**
import Element from "/js/classes/element.js"
import NavIcon from "./header-nav-icon.js"
export default class {
constructor(publisher) {
return new Element("div", {
id: "header",
traits: { publisher },
ready: self => {
self.append(new Element("div", {
selector: "title-wrapper",
classes: ["title-wrapper"],
ready: self => {
self.append(new Element("div", {
selector: "location-wrapper",
classes: ["location-wrapper"],
ready: self => {
self.traits.publisher.on("header/add-nav", data => {
self.append(new Element("div", {
selector: "location-item-wrapper",
classes: ["location-item-wrapper"],
ready: self => {
self.traits.publisher.on("header/select-nav/" + data.data.name.toLowerCase(), data => {
self.addClass("visible")
});
self.append(new Element("div", {
id: data.data.name.toLowerCase() + "-nav",
classes: ["location-item", "heading"],
text: data.data.name
}))
self.append(new Element("img", {
classes: ["location-item-icon"],
attributes: {
"src": data.data.iconPath.split(".png")[0] + "-flat.png"
}
}))
self.append(new Element("img", {
selector: "corner",
classes: ["corner"],
attributes: {
"src": data.data.cornerPath
}
}))
}
}))
})
}
}))
self.append(new Element("div", {
selector: "sub-location-wrapper",
classes: ["sub-location-wrapper", "subheading"]
}))
}
}))
self.append(new Element("div", {
selector: "nav-wrapper",
classes: ["nav-wrapper", "center-contents"],
ready: self => {
self.traits.publisher.on("header/add-nav", data => {
console.log("header/add-nav, data", data.data)
console.log("adding nav-item")
self.append(new NavIcon().use({
data: data.data
}))
});
self.append(new Element("div", {
classes: ["title-bg-wrapper"],
ready: self => {
self.append(new Element("img", {
classes: ["title-bg-icon"],
attributes: {
"src": "./assets/header.png"
}
}))
self.append(new Element("div", {
classes: ["title-bg-text"],
innerHTML: "BIDRATE <br/> RENAISSANCE"
}))
}
}))
}
}))
}
}).appendTo(document.body)
}
}
[1]: https://en.wikipedia.org/wiki/Publish%E2%80%93subscribe_pattern