JavaScript code review checklist

1. Do not discriminate errors by parsing messages
2. Avoid spreading large arrays into Math.max
3. Use structuredClone for JSON-free deep copies
4. Preserve the original error when wrapping
5. Use RangeError for invalid argument ranges
6. Handle malformed URI escapes
7. Do not rely on toString tags for type detection
8. Release stream reader locks
9. Avoid reversing a copy to find the last match
10. Check findIndex() against -1
11. Do not use toISOString for local time
12. Treat getTime() values as milliseconds

12 rules to check in every JavaScript pull request

Revix reviews JavaScript pull requests against 936 rules. This page shows a short list we picked from them. Each rule has a bad example, a good example and a source. We check each rule against its source and update the list every month. Rules marked as flagged by Revix have come up in real reviews.

Do not discriminate errors by parsing messages

Error messages are descriptive text and can be similar across different failures; the documentation recommends error cause for discriminating errors emitted by a library.

Avoid

if (err.message === "Not found") handleMissing();

Prefer

if (err instanceof NotFoundError) handleMissing();

Flagged by Revix in real reviews · Best practice · All current versions · Source: Error - JavaScript | MDN

Avoid spreading large arrays into Math.max

Argument-list expansion is constrained by the runtime's maximum number of function arguments, while reduction processes the array without expanding it.

Avoid

const highest = Math.max(...unboundedValues);

Prefer

const highest = unboundedValues.reduce((a, b) => Math.max(a, b), -Infinity);

Flagged by Revix in real reviews · Performance · All current versions · Source: Math.max() - JavaScript | MDN

Use structuredClone for JSON-free deep copies

The JSON representation cannot preserve all JavaScript object graphs or value types.

Avoid

const copy = JSON.parse(JSON.stringify(value));

Prefer

const copy = structuredClone(value);

Flagged by Revix in real reviews · API usage · All current versions · Source: JSON.stringify() - JavaScript | MDN

Preserve the original error when wrapping

The documented cause property preserves the reason for the new error and makes the original error and stack trace available to higher-level handlers.

Avoid

catch (err) { throw new Error("Request failed"); }

Prefer

catch (err) { throw new Error("Request failed", { cause: err }); }

Flagged by Revix in real reviews · Best practice · All current versions · Source: Error - JavaScript | MDN

Use RangeError for invalid argument ranges

RangeError semantically represents an argument that is not within the function's permitted range or set of values, making the failure distinguishable to callers.

Avoid

if (n < -500 || n > 500) throw new Error('out of range');

Prefer

if (n < -500 || n > 500) throw new RangeError('out of range');

Flagged by Revix in real reviews · API usage · All current versions · Source: RangeError - JavaScript | MDN

Handle malformed URI escapes

Malformed URL data can otherwise cause an uncaught exception and interrupt the surrounding operation.

Avoid

const value = decodeURIComponent(req.query.value);

Prefer

let value;
try { value = decodeURIComponent(input); } catch (e) { rejectInput(); }

Flagged by Revix in real reviews · API usage · All current versions · Source: URIError: malformed URI sequence - JavaScript | MDN

Do not rely on toString tags for type detection

Objects can define or inherit Symbol.toStringTag and thereby produce an unexpected tag, making class detection with Object.prototype.toString() unreliable.

Avoid

if (Object.prototype.toString.call(value) === "[object Date]") useDate(value);

Prefer

if (value instanceof Date) useDate(value);

Flagged by Revix in real reviews · API usage · All current versions · Source: Object.prototype.toString() - Using toString() to detect object class

Release stream reader locks

A forgotten lock prevents another reader from consuming the stream and can cause a TypeError when later code calls getReader().

Avoid

const reader = stream.getReader();
const chunk = await reader.read();
return chunk;

Prefer

const reader = stream.getReader();
try { return await reader.read(); } finally { reader.releaseLock(); }

Flagged by Revix in real reviews · API usage · All current versions · Source: JavaScript resource management

Avoid reversing a copy to find the last match

Reversing or copying an array adds allocation and traversal overhead that findLast() avoids.

Avoid

[...tasks].reverse().find(task => task.completed)

Prefer

tasks.findLast(task => task.completed)

Flagged by Revix in real reviews · Performance · All current versions · Source: Array.prototype.findLast() - JavaScript | MDN

Check findIndex() against -1

JavaScript treats 0 as falsy and -1 as truthy, so a direct conditional on the returned index produces incorrect control flow.

Avoid

if (items.findIndex(isMatch)) handle(items);

Prefer

if (items.findIndex(isMatch) !== -1) handle(items);

Flagged by Revix in real reviews · API usage · All current versions · Source: Array.prototype.findIndex() - JavaScript | MDN

Do not use toISOString for local time

The method converts the value to universal time rather than preserving the host's local timezone.

Avoid

const displayed = date.toISOString(); // expected local time

Prefer

const displayed = date.toLocaleString();

Flagged by Revix in real reviews · API usage · All current versions · Source: Date.prototype.toISOString() - JavaScript | MDN

Treat getTime() values as milliseconds

A unit mismatch can produce timestamps that are off by a factor of 1,000 while still looking like valid numbers.

Avoid

const expiresAt = new Date().getTime();
setTimeoutAt(expiresAt); // expects seconds

Prefer

const expiresAt = Math.floor(Date.now() / 1000);
setTimeoutAt(expiresAt);

Flagged by Revix in real reviews · API usage · All current versions · Source: Date.prototype.getTime() - JavaScript | MDN

How Revix applies these rules in pull requests

Revix matches these rules against the code each pull request changes. When a comment rests on one, it names the rule, so the author can read the reasoning and the source instead of taking the bot's word for it.

+if (err.message === "Not found") handleMissing();

revixappdevBot

suggestion: Error messages are descriptive text and can be similar across different failures; the documentation recommends error cause for discriminating errors emitted by a library.

📚 Rule (JavaScript): Do not discriminate errors by parsing messages

Reviews run natively on GitHub, GitLab and Bitbucket.

Related checklists

• Next.js code review checklist
• Python code review checklist
• React code review checklist
• TypeScript code review checklist