Skip to content

Commit

Permalink
Improve documentation and taxonomy of all Javascript rules
Browse files Browse the repository at this point in the history
  • Loading branch information
utarwyn committed Dec 3, 2023
1 parent ae1b8cd commit 5ed903a
Show file tree
Hide file tree
Showing 18 changed files with 304 additions and 129 deletions.
6 changes: 6 additions & 0 deletions ecocode-rules-specifications/src/main/rules/EC11/EC11.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{
"title": "Disallow multiple access of same DOM element.",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,36 @@
This rule aims to reduce DOM access assigning its object to variable when access multiple time.
It saves CPU cycles.
:!sectids:

== Examples
== Why is this an issue?

Examples of **incorrect** code for this rule:
Accessing the Document Object Model (DOM) is a relatively expensive operation in terms of performance.
Each time you access the DOM, the browser needs to traverse the document tree to find the requested element.
By assigning the DOM object to a variable when accessed multiple times, you avoid redundant traversals, leading to improved performance.

[source,js]
Assigning the DOM object to a variable not only improves performance but also enhances code readability.
It makes the code more concise and self-explanatory.
Developers reading the code can understand that the variable holds a reference to a specific DOM element, and its subsequent use is likely for multiple operations.

Here's an example in JavaScript to illustrate this rule:

[source,js,data-diff-id="2",data-diff-type="noncompliant"]
----
var el1 = document.getElementById("block1").test1;
var el2 = document.getElementById("block1").test2;
const width = document.getElementById('block').clientWidth;
const height = document.getElementById('block').clientHeight; // Non-compliant
----

Examples of **correct** code for this rule:

[source,js]
[source,js,data-diff-id="1",data-diff-type="noncompliant"]
----
var blockElement = document.getElementById("block1");
var el1 = blockElement.test1;
var el2 = blockElement.test2;
const blockElement = document.getElementById('block'); // Compliant
const width = blockElement.clientWidth;
const height = blockElement.clientHeight;
----

In the first example, getElementById is called twice, potentially resulting in two separate traversals of the DOM tree.
In the second example, the DOM element reference is cached in the `blockElement` variable, and subsequent property accesses use this cached reference.

== Resources

=== Documentation

- https://github.com/cnumr/best-practices/blob/main/chapters/BP_054_en.md[CNUMR best practices] - Reduce DOM access via JavaScript
- https://developer.mozilla.org/en-US/docs/Learn/Performance/JavaScript#tips_for_writing_more_efficient_code[Mozilla Web Technology for Developers] - Tips for writing more efficient code
6 changes: 6 additions & 0 deletions ecocode-rules-specifications/src/main/rules/EC12/EC12.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{
"title": "Disallow multiple style changes at once.",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
This rule aims to disallow batching multiple style changes at once.
:!sectids:

To limit the number of repaint/reflow, it is advised to batch style modifications by adding a class containing all style changes that will generate a unique reflow.
== Why is this an issue?

== Examples
Browsers optimize rendering performance by batching and combining similar operations.
However, when making multiple CSS changes at once, it can disrupt the browser's optimization mechanisms.
Applying changes individually allows the browser to better optimize the rendering process.

Examples of **non-compliant** code for this rule:
Making multiple CSS changes in a single batch can trigger multiple reflows and repaints in the browser.
Reflows and repaints are resource-intensive operations that can lead to performance issues.
Applying changes individually minimizes the number of reflows and repaints, improving overall page performance.

[source,html]
Here's an example in JavaScript and CSS to illustrate this rule:

[source,html,data-diff-id="3",data-diff-type="noncompliant"]
----
<script>
element.style.height = "800px";
element.style.width = "600px";
element.style.color = "red";
element.style.width = "600px"; // Non-compliant
element.style.color = "red"; // Non-compliant
</script>
----

Examples of **compliant** code for this rule:

[source,html]
[source,html,data-diff-id="10",data-diff-type="compliant"]
----
<style>
.in-error {
Expand All @@ -28,6 +32,14 @@ Examples of **compliant** code for this rule:
</style>
<script>
element.addClass("in-error");
element.addClass("in-error"); // Compliant
</script>
----

In the first example, multiple CSS properties are set in a single batch, while in the second example, changes are applied through a CSS class.

== Resources

=== Documentation

- https://github.com/cnumr/best-practices/blob/main/chapters/BP_045_en.md[CNUMR best practices] - Modify several CSS properties at once
8 changes: 7 additions & 1 deletion ecocode-rules-specifications/src/main/rules/EC13/EC13.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{
"title": "Prefer API collections with pagination.",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand All @@ -12,7 +18,7 @@
"nestjs",
"performance"
],
"defaultSeverity": "Minor",
"defaultSeverity": "Major",
"compatibleLanguages": [
"TYPESCRIPT"
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
This rule aims to reduce the size and thus the network weight of API returns that may contain many elements. This rule is built for the https://nestjs.com[NestJS framework] but can work with a controller `@Controller()` and a decorated method `@Get()`.
:!sectids:

== Examples
== Why is this an issue?

Examples of **non-compliant** code for this rule:
Pagination helps in optimizing the performance of API requests, especially when dealing with large datasets.
Instead of retrieving the entire dataset in a single request, pagination allows fetching a smaller subset of data, reducing the response time and resource usage.

[source,typescript]
Fetching only the necessary data reduces the amount of data transmitted over the network.
This is particularly important for users on limited bandwidth or mobile devices.
Pagination ensures that only the relevant data is transferred, conserving bandwidth and improving overall network efficiency.

This rule is built for the https://nestjs.com[NestJS framework] but can work with a controller `@Controller()` and a decorated method `@Get()`.

[source,typescript,data-diff-id="4",data-diff-type="noncompliant"]
----
@Controller()
class Test {
@Get()
public find(): Promise<string[]> {}
public find(): Promise<string[]> {} // Non-compliant
}
----

Examples of **compliant** code for this rule:

[source,typescript]
[source,typescript,data-diff-id="10",data-diff-type="compliant"]
----
interface Pagination {
items: string[];
Expand All @@ -26,6 +31,18 @@ interface Pagination {
@Controller()
class Test {
@Get()
public find(): Promise<Pagination> {}
public find(): Promise<Pagination> {} // Compliant
}
----

== Resources

=== Documentation

- https://github.com/cnumr/best-practices/blob/main/chapters/BP_076_en.md[CNUMR best practices] - Avoid transferring large amounts of data for processing tasks
- https://github.com/ppetzold/nestjs-paginate[nestjs-paginate] - Pagination and filtering helper method using Nest.js framework

=== Articles & blog posts

- https://nordicapis.com/optimizing-the-api-response-package/[How to Optimize the API Response Package]
- https://dev.to/pragativerma18/unlocking-the-power-of-api-pagination-best-practices-and-strategies-4b49[Unlocking the Power of API Pagination: Best Practices and Strategies]
10 changes: 8 additions & 2 deletions ecocode-rules-specifications/src/main/rules/EC24/EC24.json
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
{
"title": "Limit the number of returns for a SQL query",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
"constantCost": "15min"
},
"tags": [
"ecocode",
"eco-design",
"performance",
"sql"
],
"defaultSeverity": "Minor",
"defaultSeverity": "Major",
"compatibleLanguages": [
"JAVASCRIPT",
"TYPESCRIPT"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,34 @@
This rule aims at reducing CPU consumption by limiting the number of returns for a single SQL query.
:!sectids:

== Examples
== Why is this an issue?

Examples of **non compliant** code for this rule:
SQL queries often involve processing large amounts of data, and fetching a large number of rows can consume significant CPU resources.
By limiting the number of rows returned, you reduce the amount of processing that needs to be done by the database engine, which in turn lowers CPU consumption.

[source,js]
Transmitting a large number of rows over a network can be resource-intensive.
By restricting the result set size, you reduce the amount of data that needs to be transferred between the database and the application, improving network efficiency.

If you store data about customers, you certainly don’t need to retrieve information of all at once, because the larger the table will be, the more elements the query will return.

[source,js,data-diff-id="1",data-diff-type="noncompliant"]
----
const query = "SELECT * FROM clients";
const query = "SELECT * FROM customers"; // Non-compliant
----

Examples of **compliant** code for this rule:
It may therefore be a good idea to limit the results and use pagination, for example.

[source,js]
[source,js,data-diff-id="1",data-diff-type="compliant"]
----
const query = "SELECT columns FROM table_name FETCH FIRST number ROWS ONLY";
const query = "SELECT id,name,email FROM customers FETCH FIRST 10 ROWS ONLY"; // Compliant
----

== Resources

=== Documentation

- https://dev.mysql.com/doc/refman/8.0/en/limit-optimization.html[MySQL Reference Manual] - LIMIT Query Optimization
- https://www.postgresql.org/docs/current/queries-limit.html[PostgreSQL: Documentation] - LIMIT and OFFSET

=== Articles & blog posts

- https://www.oreilly.com/library/view/high-performance-mysql/9780596101718/ch04.html[Query Performance Optimization]
6 changes: 6 additions & 0 deletions ecocode-rules-specifications/src/main/rules/EC25/EC25.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{
"title": "Do not use an image with empty source attribute",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "LOW"
},
"attribute": "EFFICIENT"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,45 @@
This rule aims to prohibit the usage of <img/> without specifying its source attribute because it causes extra and unnecessary http requests. This rule is build for the https://react.dev/[react library] and JSX.
:!sectids:

== Examples
== Why is this an issue?

Examples of **non compliant** code for this rule:
When the src attribute is missing, some browsers may still attempt to make a request to the current URL (the base URL of the document) in an attempt to fetch the image.
This can lead to unnecessary server requests, impacting performance and potentially causing errors.

[source,html]
Proper use of the src attribute is essential for web accessibility.
Screen readers and other assistive technologies rely on valid image sources to provide meaningful information to users with disabilities.
A missing src attribute can result in confusion and hinder accessibility.

[source,typescriptjsx,data-diff-id="3",data-diff-type="noncompliant"]
----
<img src='' />
<img/>
return (
<>
<img src="" /> // Non-compliant
<img /> // Non-compliant
</>
)
----

Examples of **compliant** code for this rule:
The HTML specification requires the src attribute for the <img> element, and not including it may lead to non-compliance with standards.

[source,js]
[source,typescriptjsx,data-diff-id="4",data-diff-type="compliant"]
----
import imgSvg from './img.svg'
<img src='./img.svg' />
<img src={imgSvg} />
import myLogo from "./logo.svg"
return (
<>
<img src="./logo.svg" /> // Compliant
<img src={myLogo} /> // Compliant
</>
)
----

This rule is build for https://react.dev/[React] and JSX.

== Resources

=== Documentation

- https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/Images_in_HTML[Mozilla Web Technology for Developers] - Images in HTML

=== Articles & blog posts

- https://humanwhocodes.com/blog/2009/11/30/empty-image-src-can-destroy-your-site/[Empty image src can destroy your site]
6 changes: 6 additions & 0 deletions ecocode-rules-specifications/src/main/rules/EC29/EC29.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{
"title": "Avoid usage of CSS animations",
"type": "CODE_SMELL",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
Expand Down
Loading

0 comments on commit 5ed903a

Please sign in to comment.