top | item 14700741

A set of best practices for JavaScript projects

265 points| Liriel | 8 years ago |github.com | reply

145 comments

order
[+] zer0tonin|8 years ago|reply
> Always use //todo: comments to remind yourself and others about an unfinished job

No please don't do this, no one will ever do the todo. The code base I'm currently working on has more than 2000 todos.

Instead of this, create a ticket in Jira, Asana or whatever issue tracker you prefer. This way it can be easily taken in account when doing a sprint planning and even if no one opens the problematic file ever again, the issue will still be visible.

[+] ganonm|8 years ago|reply
This is misleading advice. Most linters can be configured to fail upon detecting a 'TODO'.

I make liberal use of TODOs when developing, typically for reminding myself to e.g. localize a string I've hardcoded during prototyping or to refactor kludgy code.

A pre commit git hook gets triggered to run the linter when I attempt to commit which helps avoid the possibility of adding code containing 'TODOs'. Obviously I can just add the '-n' flag to not trigger hooks, but this requires intention.

In short, using TODOs appropriately is a massive boost to productivity and the alternative is either to just do those random tasks that pop into your head during the course of writing code or to just hope that you remember to do it later.

[+] andreasklinger|8 years ago|reply
Your pov is understandable but i treat it differently.

They are essentially comments that explain missing functionality. You shouldn't add future features or random ideas but obvious missing functionality or workarounds that happen at this place.

i also prefer to leave my name b/c blame never works as it should. it's not the name of the person doing the todo but writing it

    // todo(andreasklinger): feel free to remove this when the normalization api 
    //   workaround (see fileX.js) is no longer needed
Related: https://dev.to/andreasklinger/comments-explain-why-not-what-...
[+] Kiro|8 years ago|reply
I prefer TODOs. At the places I've worked filing an issue was a sure way to bury something to never be seen again. You file-and-forget. At one point we even wiped the tracker just because it had so many issues. Definitely a management problem but still a true story.

How do you know that a function is unfinished if you have thousands of issues? Adding a TODO puts it in your face so pretty hard to miss.

[+] underwater|8 years ago|reply
A variant I've seen is //TODO(#7362) where the number is a ticket and is enforced by a lint rule.
[+] lucideer|8 years ago|reply
> No please don't do this, no one will ever do the todo.

I've worked in numerous places where TODO placeholders were regularly processed by devs as an explicitly scheduled exercise. If the TODOs are being left to rot, this seems a symptom of bad dev process management, rather than being inherent to the pattern itself.

> create a ticket

While I've never seen it rigidly enforced, most TODOs I've worked with have been linked to relevant tickets (with ticket #number being referenced in the TODO comment).

It's also clearly common enough practice to be supported out-of-the-box without config both by many IDE syntax highlighters and also, I think, by some linters.

[+] e12e|8 years ago|reply
At least, today, if you want // TODO or // FIXME - do both :add a ticket number, like: // TODO Refactor for Greater Good #123 (or https://<github>|<jira>/#123.

Filling in issues can feel like "busy work" - but it really is the least bad way to split up work, and keep track of things.

I think fossil has the right idea here, distributing issues along with the source code repository (and there's a few attempts to bring that to git/mercurial - but I've yet to find one that really works).

In general project "meta data" like documentation and tickets needs to be kept in a database, and there needs to be cross-reference between code revisions and ticket state as well as wiki/documentation - but they probably are 2 or 3 "different" projects.

[+] com2kid|8 years ago|reply
> No please don't do this, no one will ever do the todo. The code base I'm currently working on has more than 2000 todos.

Visual Studio integrates TODOs into its UI. After rolling off of feature work I can pull up the TODO list in VS and start picking tasks off, jumping right into the code.

[+] saltedmd5|8 years ago|reply
What exactly is an "unfinished job"?

If it's functionality that doesn't exist yet, it doesn't need to exist until there is something that requires it, in which case a TODO is useless and your ticket will be a meaningful business requirement if and when that requirement exists.

If it's "tech debt," then you have to ask yourself "can it ship like this?" If they answer to that question is "yes," then you should really be asking yourself "does this need doing?"

If at that point the answer really is "yes" (unlikely) then hopefully your team has enough context and understanding of their codebase that they can do it when appropriate - if you really really have to, raise a ticket.

TODOs are a symptom that something is wrong up the chain.

[+] aequitas|8 years ago|reply
Like said already create a ticket instead of a to-do.

But also, instead if writing todos, explain the context and reasoning behind what needs to be done instead. In the future when this code is visited again this will help you or others to get into the right mode to make a proper decision about what to do with this code. Maybe the reason to do something is no longer valid, if the intention is properly documented the decision can be made at that point. This allows for proper refactoring instead of skipping todos and keeping code for legacy reasons.

So instead of: //todo: remove this code

Do: // required for legacy API clients, should be removed after last client is deprecated.

[+] dspillett|8 years ago|reply
Do both and reference the ticket in the comment (something like "// TODO: deal properly with NULLs here, see Jira://PRJ-123" with the ticket going into as much extra detail as it relevant).

That way someone looking at the code can see that it is not in its intended final form rather than having to intuit the fact from other clues or knowing every ticket in the work list. Particularly useful if trying to find/diagnose a bug that may be caused/worsened/highlighted by the code in question. Also works in reverse: of the ticket doesn't give accurate location information for the related code you can search for the ticket ID and find where you should start looking from the comments.

[+] lhnz|8 years ago|reply
On the other hand, if you are using `TODO` comments within your code, a good idea is to place something like the `fixme` CLI [0] into a prepush hook [1] so that it is always highly visible.

The length of this output then acts as an intuitive measure of accrued technical debt and can be leveraged within a team to get time to fix the underlying problems.

[0] https://github.com/JohnPostlethwait/fixme

[1] https://github.com/typicode/husky

[+] Pigo|8 years ago|reply
On a side note, I really can't stand Jira, and don't understand why it's become so popular. Even visual studio online is far less cumbersome, and easier to navigate.
[+] Already__Taken|8 years ago|reply
The main issue with that is you can't just clone the repo and walk off with the list of things to work on, you ALSO need the ticketing system constantly accessible and up to date to reflect the code you're working with. but it does seem way neater and easier to use the issue tracking for this purpose, right tool for the job.
[+] dsgriffin|8 years ago|reply
Also "unfinished jobs" shouldn't be getting merged in the first place. If you see something unrelated to the feature you're working on in that particular commit, make a ticket on JIRA/Other and address it in a separate commit/PR.
[+] dpix|8 years ago|reply
I came here to write this exact same comment. If people insist on having todos in code. I make sure they at least reference a jira ticket.

The code should describe your business logic, not the work to do to build that code. What if no one looks at that file for a year?

[+] mmcnl|8 years ago|reply
Agreed. The more simple answer is to never commit unfinished or broken code. Inline todo's are a definitive no-go. This implies a lack of control of workload management.
[+] z3t4|8 years ago|reply
Or better, don't add more tech debt!! The todo-list will just keep growing and for every todo it will slow down further development. Just do the extra work.
[+] taylodl|8 years ago|reply
This reminds me of the old saying "if you don't have time to do it now what makes you think you'll ever have the time to do it later?"
[+] znpy|8 years ago|reply
"Tools, not policies".

I must say, this page reminds me of how lightyears ahead Perl was/is in terms of tooling.

For example: are you coding a package? Before even starting brainstorming, just boostrap your package with ExtUtils::MakeMaker. Long story short, that package will boostrap a mostly blank (but customizable via various command-line switches) Perl package with most things you will need for development and distribution: a directory structure, a Makefile (for building and installing), some pre-set comments, mostly blank test files etc etc.

This is something that, for example, the Go programming language team has rightfully picked up in terms of directory structure conventions and tooling (gofmt).

[+] grenoire|8 years ago|reply
JS ecosystem tries to do that by having bootstrapping repositories.

Although I will have to say that it is nowhere near as high-quality and convenient as Go has it. Standardising best practices most often seem too restrictive to programmers, but Go has done an excellent job at drawing that line and staying behind it.

[+] lol768|8 years ago|reply
> I must say, this page reminds me of how lightyears ahead Perl was/is in terms of tooling.

When Perl has such a mess of a syntax that its meaning is ambiguous to any static parser, I find it difficult to believe Perl has such amazing tooling - JetBrains for one refuse to implement any official support for it (see https://youtrack.jetbrains.com/issueMobile/IDEA-87976#).

[+] mattacular|8 years ago|reply
It is a shame you have to adopt one of the larger frameworks like Angular, React, or Ember to get some tooling like this.

And certainly "gofmt" is sorely missed in every other language I've used since!

[+] rsj_hn|8 years ago|reply
This is a mishmash of coding standards (at ABC CO we use tabs!) randomly sprinkled with best practices. It's important to standardize on things without one way necessarily being better than another. The organization also seems a bit rushed. For example, not exposing the DB schema in a URL shouldn't really go into the section on encoding. Moreover, SQL injection is almost never fixed by using an escaping function, as by far the most common problem with SQL injection is letting the client specify field and column names, or things that aren't quoted (e.g. numerical id's) and so don't benefit from escaping.

I don't want to discourage any company from setting and documenting coding standards, but this particular document feels a bit rushed and could use adherence to best practices of its own. For example,

1. If making an opinionated mandate, provide a link to an explanation

2. Consistently provide links to examples

3. Maybe do some spell-checking: "Hallo"?

4. Try to maintain a uniform level of specificity

E.g. in the section on URL security, there is this vague comment: "Attackers can tamper with any part of an HTTP request, including the URL, query string,"

Which, while a true, doesn't describe what the programmer should or shouldn't do. It's just hanging there. Compare with the weird specificity of listing git commands in section 1.

That tells me the author knew a lot about checking stuff into git, but was a bit out of his/her league when it came to Security, so they just made generic security statements while giving precise instructions for using git.

[+] th4dv|8 years ago|reply
A lot of companies these days create "Best practices" guidelines just to boost their online recognition. Very often I find these guidelines useless or even harming.
[+] Sohcahtoa82|8 years ago|reply
> 3. Maybe do some spell-checking: "Hallo"?

I figured the string was German.

[+] YCode|8 years ago|reply
Any recommendations on best practices for JS?
[+] a_humean|8 years ago|reply
"Use stage-1 and higher JavaScript (modern) syntax for new projects. For old project stay consistent with existing syntax unless you intend to modernise the project."

That seems doesn't seem like a good idea. I can understand recommending Stage-2 and/or Stage-3, but stage-1 seems premature. Stage-1 in the TC39 process is still proposal stage that is flagged for expecting "Major" changes in the spec and possibly the syntax of the feature. Being stage-1 also is a poor guarantee that it will progress to future stages or become part of the spec. Stage-2 at least signals that the committee think its likely to eventually become part of the spec with only minor revisions.

[+] vahidpg|8 years ago|reply
You're actually right. I will change that to stage-2. Thanks for feedback.
[+] drinchev|8 years ago|reply
> Organize your files around product features / pages / components, not Roles:

Oh really? Can someone explain why is this a good thing and what's the benefit?

[1] : https://github.com/wearehive/project-guidelines#6-structure-...

[+] andreasklinger|8 years ago|reply
you want to slice your abstractions by domain logic boundaries

essentially by "what it does, not by how it does it"

eg let's say you want to do notifications for your site.

you want to have everything related to notifications within one folder and have the outside connect to it via one api (eg component)

in backends this would enable you to easily scale in performance (eg move to microservice or separately hosted instance) or team size (isolation in code, less awareness of the overall codebase "needed")

imo the next big web framework after rails will have this setup as a default

    `app/services` instead of 
    `app/controllers, app/models, app/decorators, app/workers, app/assets, app/views, etc`
[+] lhnz|8 years ago|reply
It helps the case in which you want every feature/section of your application to be a smaller self-contained application which can be modularised and loaded into other applications and environments.

I think this is the first time I saw the approach: https://github.com/erikras/ducks-modular-redux

[+] Illniyar|8 years ago|reply
creating or updating a component is usually cross-concern, so if you want to change some functionality in a component you'll have to update it's controller,view,model etc...

If they are in the same folder, it makes moving between the relevant files for a feature easier (especially if you have a lot of components and finding the right file in your "controllers" folder means searching through a few dozen files).

It also makes reasoning about a certain feature easier - since all the relevant files are grouped next to eachother you can easily move between a feature's controller and it's view.

It also makes moving a feature easier- instead of renaming each file separately you can simply move the folder to anywhere else in the structure (for instance if you move a feature from a specific page to something more generic/shared)

[+] underwater|8 years ago|reply
I'm not so sure that's a good idea. A component is one possible interface to the business logic, but there isn't necessarily a 1:1 mapping.

Colocating data logic together makes it easier to build a data model that is logical and consistent, rather than one that is coupled to UI decisions.

[+] vahidpg|8 years ago|reply
Hi guys, I published this document. Here at http://www.wearehive.co.uk we work with different developers with various levels of experience. We had to come up with a set of rules to bring some consistency to what we produce and make it easier for others to pick up and maintain it. It does look random and it's not perfect, that's why we decided to make it public and receive some feedback. I've already made notes from some of these comments. If you think something is wrong and shouldn't be there, make a pull request, or just let me know if you're too busy to do that.
[+] Azeralthefallen|8 years ago|reply
Can someone please explain to me the rational for shoving your tests in the same place as your code? I have seen this before, and every time i have tried it on anything beyond a simple project it has ended up becoming a nightmare.

Often times you have numerous test specific helper functions, and tons of other test oriented cruft, where do all these things go? The root of the project? Also where do tests that encapsulate logic between multiple components go?

[+] YCode|8 years ago|reply
One issue that is sort of addressed in this that I've been trying to find a solid answer on is how do you strongly ensure a given NPM package and its dependencies are safe to use?

So far the only real solution I've come up with is to do a signals review (github? issues? downloads/day/ etc...) and then review the code block by block carefully examining any complex or potentially dangerous code.

It works, and you learn a lot about the packages and why they depend on each other, but at the same time this process is exhausting.

What would be a godsend is some major security brand who can vouch (even at cost) for a given version of major packages including their dependency tree.

[+] nscalf|8 years ago|reply
Well this is definitely one way to do things... The reality of it is if you're making a project, and everyone is on board with the standards of how to do things and it's not making it unnecessarily confusing, you're practicing good coding. For example, if you have your tests isolated to a testing folder vs in a product/feature folder and everyone is on board, then it doesn't make any difference. I'll just look in the test folder for the tests rather than the feature folder.

Honestly, I don't get the obsession and combativeness around "best practices". Best practices are whatever your team uses effectively to get shit done.

[+] sAbakumoff|8 years ago|reply
A set of best practices? It rather looks like a set of random thoughts.
[+] baron816|8 years ago|reply
I can't say if these are the BEST practices for JS, or if even such practices could exist. But I do think that every company should have a standard set of written out practices/guidelines and publish them. They should probably even include them in their job listings. I hope people will comment on the potential pitfalls of doing so, but a benefit would be that new and prospective engineers would have a clearer idea of what writing code for the company would be like.
[+] flavio81|8 years ago|reply
> "Only use nouns in your resource URLs, avoid endpoints like /addNewUser or /updateUser . Also avoid sending resource operations as a parameter. Instead explain the functionalities using HTTP methods:

GET Used to retrieve a representation of a resource. POST Used to create new new resources and sub-resources PUT Used to update existing resources PATCH Used to update existing resources. PATCH only updates the fields that were supplied, leaving the others alone DELETE Used to delete existing resources"

This used to be hip and cool and "look ma, i'm modern, i'm doing it REST-style!", but i don't think the fad holds anymore.

In real life, you are going to do much more "operations" to a "resource" than just "retrieve", "create", "update", or "delete", and thus you need to use operation names. It makes more sense to always send the operation name and to standarize on operation names. Also operation names can be more explicit and unambiguous.

It is, for me, a very bad idea to mix up your business model semantics (actions performed on the business model) with the semantics that belong to the OSI-model application layer (HTTP methods: GET, POST, PUT, PATCH...).

[+] ricardobeat|8 years ago|reply
How do you collaborate on a branch using this workflow? The constant rebasing means you can't have multiple devs pushing to the same branch.
[+] deadwisdom|8 years ago|reply
"Keep README.md updated as project evolves"

Sure man.

[+] okket|8 years ago|reply
And this is why I like opinionated frameworks like Ember.js so much. Most of the best practices are baked in. Plus you usually do not encounter a random subset of 'best practices' with added personal preferences/exceptions in other peoples projects, making collaboration much easier.
[+] tieTYT|8 years ago|reply
> Place your test files next to the implementation.

Java background is probably biasing me, but I don't like this. It interferes with my ability to find code because the file list in every directory is twice as large.

What are the benefits of putting it together?

[+] jroseattle|8 years ago|reply
"While developing a new project is like rolling on a green field for you, maintaining it is a potential dark twisted nightmare for someone else."

There is nothing about this list of best practices that makes any applicable project more maintainable without understanding the practices being followed.

As long as these practices are well-known within the company and enforced over the lifecycle of code/product/project/etc., it will succeed. Without that understanding, this list eventually becomes untrusted.

Good on the folks at hive for following what appears to be solid practices in their JS projects.

[+] Principe|8 years ago|reply
I hadn't noticed before but... Please do not recommend that people use "stage-1 features". Maybe if it is your own money, but not if you do it with other people's money.
[+] cottsak|8 years ago|reply
_facepalm_ ugh! Gitflow again! Get on board with Github Flow people - it's going to save you so much pain