Skip to content

Commit

Permalink
[Style guides]: Update style guides with docs & optional methods (#53)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivadimko authored Dec 5, 2024
1 parent 4f68d38 commit 99aa44b
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 21 deletions.
134 changes: 118 additions & 16 deletions javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@
- [12. Properties](#12-properties)
- [12.1. Use dot notation when accessing properties.](#121-use-dot-notation-when-accessing-properties)
- [12.3. Use bracket notation `[]` when accessing properties with a variable.](#123-use-bracket-notationwhen-accessing-properties-with-a-variable)
- [12.3. Use exponentiation operator `**` when calculating exponentiations.](#123-use-exponentiation-operatorwhen-calculating-exponentiations)
- [13. Variables](#13-variables)
- [12.4 Disallow optional fields that change method behavior](#124-disallow-optional-fields-that-change-method-behavior)
- [13. Variables](#13-variables)
- [13.1. Always use `const` or `let` to declare variables. Not doing so will result in global variables. We want to avoid polluting the global namespace. Captain Planet warned us of that.](#131-always-useconstorletto-declare-variables-not-doing-so-will-result-in-global-variables-we-want-to-avoid-polluting-the-global-namespace-captain-planet-warned-us-of-that)
- [13.3. Use one `const` or `let` declaration per variable or assignment.](#133-use-oneconstorletdeclaration-per-variable-or-assignment)
- [13.3. Group all your `const`s and then group all your `let`s.](#133-group-all-yourconsts-and-then-group-all-yourlets)
Expand Down Expand Up @@ -121,7 +121,8 @@
- [18.4. Prefixing your comments with `FIXME` or `TODO` helps other developers quickly understand if you’re pointing out a problem that needs to be revisited, or if you’re suggesting a solution to the problem that needs to be implemented. These are different than regular comments because they are actionable. The actions are `FIXME: -- need to figure this out` or `TODO: -- need to implement`.](#184prefixing-your-comments-withfixmeortodohelps-other-developers-quickly-understand-if-youre-pointing-out-a-problem-that-needs-to-be-revisited-or-if-youre-suggesting-a-solution-to-the-problem-that-needs-to-be-implemented-these-are-different-than-regular-comments-because-they-are-actionable-the-actions-arefixme----need-to-figure-this-outortodo----need-to-implement)
- [18.5. Use `// FIXME:` to annotate problems.](#185use-fixmeto-annotate-problems)
- [18.6. Use `// TODO:` to annotate solutions to problems.](#186use-todoto-annotate-solutions-to-problems)
- [19. Whitespace](#19-whitespace)
- [18.7. Fields in Sequelize models and business logic methods should have a `@description` annnotation with JSDoc comment](#187-fields-in-sequelize-models-and-business-logic-methods-should-have-a-description-annnotation-with-jsdoc-comment)
- [19. Whitespace](#19-whitespace)
- [19.1. Use soft tabs (space character) set to 2 spaces.](#191use-soft-tabs-space-character-set-to-2-spaces)
- [19.2. Place 1 space before the leading brace.](#192place-1-space-before-the-leading-brace)
- [19.3. Place 1 space before the opening parenthesis in control statements (`if``while` etc.). Place no space between the argument list and the function name in function calls and declarations.](#193place-1-space-before-the-opening-parenthesis-in-control-statements-ifwhileetc-place-no-space-between-the-argument-list-and-the-function-name-in-function-calls-and-declarations)
Expand Down Expand Up @@ -2092,25 +2093,54 @@ function getProp(prop) {
const isJedi = getProp('jedi');
```

#### 12.4 Disallow optional fields that change method behavior

💡 Note: it is related only to fields that impact behavior inside methods. In other words - if there is `if` in the code, related to the field, the field should be required

#### 12.3. Use exponentiation operator `**` when calculating exponentiations.
>❓Why? While it is generally recommended to avoid params that change method behavior, sometimes it's not possible to avoid. For such cases, having required fields helps to address all places where the method is used and update them accordingly. If the field is optional, it's easy to forget about it, leading to bugs
eslint: [`no-restricted-properties`](https://eslint.org/docs/rules/no-restricted-properties)



```javascript
```typescript
// ❌ bad
const binary = Math.pow(2, 10);
const getNextClosestWeekday = (options: {
// optional variable
keepReferenceTime?: boolean
}): Date {
let closestWeekdayDate = // logic here

// changed behavior. easy to forget about if keepReferenceTime is not passed
if (!keepReferenceTime) {
closestWeekdayDate = closestWeekdayDate.set({
hour: 0,
minute: 0,
second: 0,
millisecond: 0,
});
}
}

// ✅ good
const binary = 2 ** 10;
```

const getNextClosestWeekday = (options: {
// requierd variable
keepReferenceTime: boolean
}): Date {
let closestWeekdayDate = // logic here

// changed behavior. As variable is always passed, it's obvious what's changed inside
if (!keepReferenceTime) {
closestWeekdayDate = closestWeekdayDate.set({
hour: 0,
minute: 0,
second: 0,
millisecond: 0,
});
}
}

#### 13\. Variables
```

13\. Variables
----------



Expand Down Expand Up @@ -3079,17 +3109,89 @@ class Calculator extends Abacus {
```


#### 18.7. Fields in Sequelize models and business logic methods should have a `@description` annnotation with JSDoc comment

#### 19\. Whitespace
>❓Why? It gives more context right at the moment of reading code. Especially useful for people working with other teams' code. Extremely useful for QAs, Data analytics, new Mates and others who don't work with particular module day to day but needs context.

>❓What is "Business logic method"? In short, if method requires domain context, it is considered as the business logic one. There is no need to write description to "sum" helper

Example of business logic method:
**/modules/subscription/subscription.service.ts**
```typescript
/**
* @description Payment is "intro" if it is a FIRST payment and
* payment_type is TRIAL or INTRO_OFFER. Usually it means this payment is processed
* with a discount.
*/
async isIntroPayment(
options: {
// list of options
},
): Promise<boolean> {
// rest of code here
}
```

**models/Subscription.ts**
```typescript
// ❌ bad
export class Subscription extends ModelBase<User> {
@Column
started: boolean;
@Column({
type: DataType.DOUBLE,
})
price: number;
@ForeignKey(() => Currency)
@Column({
field: 'currency_id',
})
currencyId: number;
}
// ----
// ✅ good
export class Subscription extends ModelBase<User> {
/**
* @description Is subscription started. If true, user has access to the subscription benefits.
* Is set up after the first payment.
*/
@Column
started: boolean;
/**
* @description Subscription price. Copied on creation from the SubscriptionPlanPricingOption or from the SubscriptionPlan
* if the pricing option is not set.
* It allows to store the price at the moment of the subscription creation and not to change it on the plan changes.
*/
@Column({
type: DataType.DOUBLE,
})
price: number;
/**
* @description Subscription currency id. Related to the Currency model.
* Copied on creation from the SubscriptionPlanPricingOption or from the SubscriptionPlan
* if the pricing option is not set.
* It allows to store the currency at the moment of the subscription creation and not to change it on the plan changes.
*/
@ForeignKey(() => Currency)
@Column({
field: 'currency_id',
})
currencyId: number;
}
```

19\. Whitespace
----------

#### 19.1. Use soft tabs (space character) set to 2 spaces.

eslint: [`indent`](https://eslint.org/docs/rules/indent.html)



```javascript
// ❌ bad
function foo() {
Expand Down
10 changes: 5 additions & 5 deletions mate-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,26 @@

[go/mate-api-style-guide](http://go/mate-api-style-guide)

- [1\. Data fetching](#1-data-fetching)
- [1. Data fetching](#1-data-fetching)
- [1.1. Use repository instead of explicit ORM methods to fetch data from DB](#11-use-repository-instead-of-explicit-orm-methods-to-fetch-data-from-db)
- [1.2. Always use plain objects instead of ORM instances. Avoid using ORM specific methods outside of the repository](#12-always-use-plain-objects-instead-of-orm-instances-avoid-using-orm-specific-methods-outside-of-the-repository)
- [1.3. Use `find` and `get` prefixes to specify whether method returns `null` or throws an Error](#13-use-find-and-get-prefixes-to-specify-whether-method-returns-null-or-throws-an-error)
- [1.4. Use data-loaders instead of ORM methods in `find` and `get` methods](#14-use-data-loaders-instead-of-orm-methods-in-find-and-get-methods)
- [1.5. Store common entity related errors in constants](#15-store-common-entity-related-errors-in-constants)
- [2\. GraphQL](#2-graphql)
- [2. GraphQL](#2-graphql)
- [2.1. Always include modified objects in mutation responses](#21-always-include-modified-objects-in-mutation-responses)
- [2.2. Do not add foreign keys to the GraphQL schema until it's really needed](#22-do-not-add-foreign-keys-to-the-graphql-schema-until-its-really-needed)
- [2.3. Use @Query, @Mutation, @Child decorators while creating resolvers](#23-use-query-mutation-child-decorators-while-creating-resolvers)
- [2.4. Write resolver class name in Uppercase](#24-write-resolver-class-name-in-uppercase)
- [2.5. Resolver name should end with Resolver, not Query/Mutation/Child](#25-resolver-name-should-end-with-resolver-not-querymutationchild)
- [3\. Models](#3-models)
- [3. Models](#3-models)
- [3.1. Extend `ModelBase` while writing new Model](#31-extend-modelbase-while-writing-new-model)
- [3.2. Nullable fields should be marked explicitly](#32-nullable-fields-should-be-marked-explicitly)
- [3.3. Related model should **always** be marked as nullable even if foreign key has `not_null` constraint](#33-related-model-should-always-be-marked-as-nullable-even-if-foreign-key-has-not_null-constraint)
- [4. Database](#4-database)
- [4.1. Use seeders for modifying data and migrations for modifying DB structure](#41-use-seeders-for-modifying-data-and-migrations-for-modifying-db-structure)
- [4.2. Follow naming template for seeders and migrations](#42-follow-naming-template-for-seeders-and-migrations)
- [4.3. Use Enum data type for enumerable values instead of String + Check Constraint](#43-use-enum-data-type-for-enumerable-values-instead-of-string--check-constraint)
1\. Data fetching
-----------------

#### 1.1. Use repository instead of explicit ORM methods to fetch data from DB
Expand Down Expand Up @@ -542,7 +542,6 @@ export class User extends ModelBase<User> {
@HasMany(() => TypingSpeedTest)
typingSpeedTests: TypingSpeedTest[];
}

// ----

console.log(User.domain) // 'undefined' or 'null'
Expand All @@ -568,6 +567,7 @@ if (User.typingSpeedTests?.length > 0) {
```

4\. Database
----------

#### 4.1. Use seeders for modifying data and migrations for modifying DB structure

Expand Down

0 comments on commit 99aa44b

Please sign in to comment.