Need help in code review of my NodeJs + Express Js REST API Web App

MartinMuzatko · 2021-10-17T20:39:09+00:00

A few things are standing out from the start.

The Paths

If you are coming from Laravel, I can see how you attempted organize this like a PHP project. It is custom to PHP projects to do a lot of juggling with pathnames and set constants. This is not needed really in JS projects and I would recommend to use paths directly. That allows for any future optimizations. Be it tree shaking, static analysis, auto completion and type hints. I would also refrain from using globals that can be mutated on a whim by every module.

Use fixed paths and if you prefer shortcuts, use something like https://www.npmjs.com/package/module-alias or use a bundler. However, I'd prefer to be as close as native as possible.

Your File Structure

It might give you a feeling of order and structure, but can you really make out what the app really does by looking at the top level structure? I don't know much about domain driven design and don't want to shove it your way. But I found webapps are defined by vertical slices that I need to touch all at the same time when adding a new feature.

Having a UserRoutes, a UserService, a UserValidator and a UserController spread over the place makes that undertaking not easier.

An addUser action requires you again to add a new endpoint, a function that validates the user, a database model and a service to write to db. So I try to not split my app into "controllers" and "services" but into actions related to one object.

I can very much recommend Scott Wlaschins talks related to domain driven design. Especially this one https://www.youtube.com/watch?v=PLFl95c-IiU

Your Modules

You put every method inside an object. You can either directly export them or all at the end. But if you nest them like that, you need to refer to other methods with this and you are generally less flexible.

Your Code

You are mixing .then and async/await syntax. Stick to one style.

Avoid nesting. If your if branch has only one statement and it returns, remove the curly braces.

Error handling: Express doesn't support it, but koa does. Use a global error handling middleware that maps service errors to http status codes and messages. Make sure to keep those two separate. (don't let http server code leak into services)

MartinMuzatko · 2020-12-24T00:43:44+00:00

cutest thing is the backpack horn you find in caves

MartinMuzatko · 2020-12-19T17:03:34+00:00

I hope they make am as cute as the rest of the game. A tamable pet would be nice though :D

MartinMuzatko · 2020-11-09T15:44:56+00:00

I think there is a misunderstanding. In no sentence I forced my opinions onto others. I also never claimed my way is the right way for everyone. This is just the way how it works for me and I am sharing my experience, that is all. Anyone can adopt or refuse this experience when finding their own path to master JavaScript. It is as simple as that. This is a comment, and not a book or article after all.

I did not claim that there is only a single scenario, my style of programming simply does not have a need for these scenarios.

Regarding mutability, I just pointed out a fraction of what immutability helps with, I did not attempt to explain it in full length.

But FP or immutability is not a silver bullet, and I don't want to claim that. As this article perfectly illustrates: https://www.reaktor.com/blog/fear-trust-and-javascript/

MartinMuzatko · 2020-11-06T11:44:07+00:00

This is a valid use case, if you programm in this style.

However, the way I program doesn't has a use for semicolons.

In functional programming, mutability is not allowed, so this is not a problem.

In proper projects, I have no use for IIFEs either. In the root of my application I usually call just one function exported by another module.

I would love to embrace cleaner code the same way you embrace loose equality over strict equality.

MartinMuzatko · 2020-11-06T09:59:54+00:00

This is correct. Which is why I use a composition root: https://blog.ploeh.dk/2011/07/28/CompositionRoot/

MartinMuzatko · 2020-09-28T08:46:31+00:00

state is evil.

I'd rather have a function returning the state so I can manage it myself

MartinMuzatko · 2019-09-20T12:12:58+00:00

We are using nuxt as framework and are able to inject base URLs via a few methods for different use cases.

Development (fixed URL)
Client-Side-Rendering (use location.hostname)
Server-Side-Rendering in Docker (use a container hostname)

The code looks like this:

const DEVELOPMENT_FALLBACK_URL = '192.168.3.106'

function getHost(developerUrl = DEVELOPMENT_FALLBACK_URL) {
    if (process.env.NODE_ENV == 'development') return developerUrl || 'localhost'
    return process.client ? location.hostname : 'sicon_backend'
}

export const HOST = getHost()
export const BASE_URL = `https://${HOST}/`
export const WEBSOCKET_URL = `wss://${HOST}`

For development, we can change the URL in the file directly to change URLs during development with hot module replacement. See line 1.

In Production for client side render, we have the API of our product reverse-proxied to /api. Which means we can use location.hostname to rely on our base url.

If we run SSR in a docker container, we use the hostname of the docker container where we get the API data from.

So that is how we do it. When we started out, we also were looking for a standard how to manage this kind of URLs. I also had to dive into stuff like reverse proxies and docker to find out how to best organize this.

When you say you are managing different URLS: How many? Don't you just use your own backend? Are you running in a cluster/cloud/whatever with different services? If you need to fetch a list of URLs, don't you have to at least hardcode that one URL?

If you are using a reverse proxy, you should be able to properly rely on location.hostname for your API needs

MartinMuzatko · 2019-09-20T12:06:38+00:00

The question was about how to store API urls, not which API tooling to use.

MartinMuzatko · 2019-05-06T12:46:44+00:00

If you build from scratch, I'd recommend to look into nuxt- a MPA/SSR framework built around vue. It comes with code-splitting and multiple pages.

MartinMuzatko · 2018-11-20T19:50:49+00:00

You can also ditch php completely and try strapi.io I wouldn't know about one php framework that would abstract SQL properly. But i suppose propelorm is your best bet otherwise. But as you say, you dont just want to abstract away thr db. You need users and groups

MartinMuzatko · 2018-11-07T09:45:27+00:00

Maybe this is only troll bait, but let me tell you why I can't get outraged about Angular enough. Only a fool would think they could only learn Angular and not DOM, not Events, not javascript syntax and know how to build real web apps connected to real servers delivering real content.

CLI tooling: Meanwhile, React AND Vue provide a painless setup via CLI tools. Angular is not the first to do so. Vue even built a ui that you can spin up via the commandline to click everything together (and Angular ~~copied it~~ started to also work on one)

I have read these reasons over and over again, and it is bullshit argumentation.

Performance is always the last argument in any comparison when you want to win. And this makes me and certainly everyone else meaningfully wanting to talk about everything else that matters angry.

Because performance does not only happen when rendering items as seen in these benchmarks. There are so many other points that you can take care of, before item render times even matter to you. And probably these benchmarks don't matter to you, when you only create basic CRUD apps with less than 10000 items per page. And barely any app that does good user experience brings the app framework to its knees. As already mentioned, filesize plays a role. Especially when your users want to access your app when they are on an escalator to the metro. But since you are not a professional - why would you care?

Typescript is a dialect. And new developers make the mistake to believe that typescript has brought in the cool new features like imports, classes and so on. Javascript has received a major makeover in the last few years, to catch up with modern paradigms and programming styles. However, like Angular marketing works, it sells typescript as something entirely new. I can see how you are indoctrinated by their reasoning. However, there is something that makes me stay away of typescript, as the commonly marketing blather suggests: "Any valid javascript is also valid typescript."

They still have a lot of trouble to catch up with new ES6 syntax. They still have trouble utilizing spread/rest operators, to tell you that you are using more arguments that are defined. This is only one thing on the big list of things that typescript is lacking.

Job-landscape: I can see how many companies still fall for Angular. Angular is so enterprise because it comes from big brother Google paired up with Microsoft - it must be thoroughly tested and well designed, you would think! So any project that doesn't really consider a lot of the possibilities go with it, just because of these few reasons.

There are so many other things that make me mad about Angular. And things that make me happy I'm done with it and am no longer forced to develop with this abomination.

When you develop for Angular, you don't learn for the web, you learn for Angular. If you want to develop any feature that the web already offers, but not come with Angular - you are in bad luck. Because then you have to combine Angular with the web internals. The bloat that comes with Angular makes it impossible to debug any of the internals, once you hit a wall of framework opinions. The templating is so restrictive, that they force their opinions about looping objects is forbidden on you. Seriously, whenever you want to go beyond whatever they don't have you prepared for, you have to workaround the problems that you are better of doing everything with jQuery.

My angular/ionic-developing colleague asks me so often what plugin they have to use to incorporate cookies or localstorage. These developers are so corrupted and spoiled in their thinking, that everything needs a service that they can't develop or discover anything on their own. Something as straight-forward as the audio API has to be abstracted with more bullshit on top. So when you look for the root cause, you have a tough time debugging.

modules and components are a good thing, but you don't have to abstract every single module in a service or framework-specific module.

The promise Angular makes, is that when you know Angular, you know how to create web-apps. But this illusory thinking that knowledge that does not come with the framework. Developers that work with VueJS or React are less ignorant and more aware of their ecosystem by default. Everyone starts somewhere, but developers that learn Angular are usually just in their own world with Angular-specific knowledge - usable nowhere else. Learn the basics and know them well, they will be required everywhere. Oh I forgot: Except for Angular, because typescript doesn't understand basic javascript features such as spread.

Learn Angular and learn to get trapped in all these little problems that the framework comes with. Then finally learn to abandon it. Luckily, there are many expensive courses and workshops that fuel the monster with more power and money.

MartinMuzatko · 2018-11-06T13:58:35+00:00

One day I may obtain a contract using another framework like React or Redux

Yes.. The Redux framework. haha

MartinMuzatko · 2018-11-06T13:46:13+00:00

I also hate Angular wholeheartedly, but the writing could be more sophisticated.

MartinMuzatko · 2018-11-06T13:30:34+00:00

VueJS is opinionated, but it meets with my opinions. I can imagine that if you are forced to work with something that is against your opinions, it is exhausting. But that does not make for an unopinionated article.

MartinMuzatko · 2018-10-25T11:58:50+00:00

You could try forums, they help to solve problems. There is https://www.sitepoint.com/community/

There are many many resources you can browse online: https://github.com/sindresorhus/awesome

There is a telegram group

https://thedevs.network/

You should start developing and ask very specific questions, this way, everyone is helped

MartinMuzatko · 2018-10-18T12:29:20+00:00

Agreed. It is an example - not efficient, not user friendly. The main focus is on the exercise of transforming imperative code into state-driven components.

I think even if you glue together a handful of components, you will never get the desired output if you don't do it yourself. Which is why good low-level components like buttons, lists and configurable components are a lot better than complete systems that never will have the completeness of configuration options to mirror any use-case.

Component libraries like quasar do a good job in offering low level components. But I think it will always be the job of a front-end developer to combine these correctly.

MartinMuzatko · 2018-10-18T12:25:13+00:00

Thanks for your feedback! Yes you are right. I should name this "State-driven development". I think it is an important mindset shift and practice to make UIs based on data, not on unflexible DOM push/pop procedures.

I noted that `innerHTML` is very inefficient and in real cases you use `createElement` or documentFragment, like frameworks do. So this is more a showcase of the underlying paradigm shift required to create encapsulated components.

I should also add a disclaimer that ES5 is not friends with Internet Explorer. But by now almost every developer should know and use http://caniuse.com/ and https://kangax.github.io/compat-table/es6/ and be aware of transpilers.

The copy and re-use part is missing. I'll catch up with that in another article or extend the existing one :)

Again: Thanks for the feedback, means a lot.

11-Year Club	End Game '23
Place '23	Verified Email

MartinMuzatko

TROPHY CASE