Coder Social home page Coder Social logo

cozy-guidelines's Introduction

Cozy Code Guidelines

Naming of Functions

To name functions, you can use some keywords that everyone will understand, like those ones :

fetchSomething : should access data by making a server side call

getSomething : should access data without making a server side call

findSomething : like the get, but with arguments

normalizeSomething : should transform data to conform to a given schema or to make sure that is coherent with other documents

saveSomething : creates or updates a document

hasSomething or isSomething : should describe a boolean variable

computeSomething : create something from other elements

makeSomething : create something from scratch

doSomethingAndForget : to convert an async func doSomething to a sync one doSomethingAndForget

ensureSomething : make sure something {is/has been} done

Naming of Queries

By "naming queries" we mean defining the as attribute of the queries options usable in cozy-client:

const queryResult = useQuery(Q(DOCTYPE), {
    as: ...,
    fetchPolicy: ...
  })

By default as must be defined according to the doctype used: as: DOCTYPE

If the query can be built with parameters (in this case id, account and date), these parameters must be included after id by preceding them with a /[PARAM_NAME]/ (note that this is unnecessary for id):

as: `${DOCTYPE}/${id}/account/${account}/date/${date}`

Import order

import libs from externals // (ex: lodash)

import libs from internal // (ex: cozy-client)

import libs from local // (absolute or relative path, depending on the project)
import styles from local // (absolute or relative path, depending on the project)

JavaScript

Return value

Avoid using undefined and prefer returning null.

Why ? undefined is used for variable that have not been assigned yet. null is for a variable with no value. See http://www.ecma-international.org/ecma-262/6.0/#sec-undefined-value, http://www.ecma-international.org/ecma-262/6.0/#sec-null-value

Promises vs async/await

Always use async / await when applicable, instead of Promises and then() chains.

Why ? Cleaner, Clearer, more readable, easier for handling errors See https://hackernoon.com/6-reasons-why-javascripts-async-await-blows-promises-away-tutorial-c7ec10518dd9

Comments

Only comments things that have business logic complexity.

Why ? Comments are an apology, not a requirement. Good code mostly documents itself. See https://github.com/ryanmcdermott/clean-code-javascript#comments

❌ Bad :

// Initiate API UserData object, for checkFormLogin token
const req1 = await request({
  url: `https://www.deezer.com/ajax/gw-light.php?method=deezer.getUserData&input=3&api_version=1.0&api_token=&cid=`
})

βœ… Good

function initiateAPIForToken () {
  return request({
    url: `https://www.deezer.com/ajax/gw-light.php?method=deezer.getUserData&input=3&api_version=1.0&api_token=&cid=`
  })
}

Date

Date management dependencies may increase app bundle and are not always useful.

In order to uniform our usage of date management package, we discourage any use of moment The main reason is that project was built for the previous era of the JavaScript ecosystem and since September 2020, for more details

Looking for alternatives, we use in that order:

Because nothing beats using native features:

Especially

  • Intl.DateTimeFormat, which provides date and time formatting
  • Intl.RelativeTimeFormat, which provides language-sensitive easy-to-read phrases for dates and timestamps

Date-fns can be used directly inside our app.

const { f } = useI18n()

The whole configuration can be found inside Cozy-UI/I18n folder

Redirections

If you need to make a link between cozy's apps, you should always use AppLinker from Cozy-UI. It handles a lot of use cases (flagship app, mobile app...). Never rely on window.location in the cozy-apps' codebase. We do a few things that can have repercussion on this value. Always use the generateWebLink method. To use it, you'll have to get the subdomainType. And to get the subdomainType, you'll need to read it from the client: const { subdomain: subDomainType } = client.getInstanceOptions()

React

Generic code style

"eslintConfig": {
    "extends": ["eslint-config-cozy-app"]
  }

Let the formatters do their jobs.

Prefer Functional Components

For any new component in our codebase, we prefer using functional component

Prefer export over export default

For exporting new component in our codebase, we prefet using export { ComponentName } over export default ComponentName. This makes maintenance easier by making renaming more explicit when importing.

React Memo

Usage of React.memo / useMemo / useCallback is quite tricky. We recommend to use React.memo wisely. We use React.memo only for medium/big component that renders often with the same props.

img

Don't use memoization if you can't quantify the performance gains.

Strictly, React uses memoization as a performance hint. While in most situations React avoids rendering a memoized component, you shouldn't count on that to prevent rendering.

Could I use Memoization?

Use the React profiler or a profiling extension to measure the benefits of applying React.memo().

Examples when to use memoization

Live example

In this example, we have an example of (not so big) component rendering a lot (because of realtime values provided by usePokemon hook).

Theoretical examples

In Cozy application, we may consider using memoization inside:

  • Page / Views are big components, rendering a lot if they depend on props.
  • slow calls to API (that lasts more than 200ms) that are not cached / using pouchdb
  • big operations (ex: increment to 10 000)

Event handling

Prefer named function instead of inline function into event handler.

With Typescript, if you have trouble with eslint rule no-misused-promises because your function is returning an Promise<void>. You can disable this rule for your specific case

Styling / theming

  • Use components from cozy-ui when possible
  • When coding a component, try to avoid stylus and prefer material UI's default solutions for theming (withStyles)

See also cozy-ui guidelines on component development.

Test libraries and tools

  • enzyme is deprecated. We use testing-library/react instead. It is always good to refactor an enzyme compliant test to a testing-library/react compliant test.
  • testcafe is deprecated.
  • We do not use snapshots anymore. Do not add new snapshots.

Tests

Unit test files

Unit test files should be located next to their source file, rather then in a subfolder (for example __tests__). This keeps them closer to their source file and encourages their maintenance.

πŸ‘ Cool

src
β”œβ”€β”€ greetings
β”‚   └── components
β”‚        └── Greeting.jsx
β”‚        └── Greeting.spec.js

❌ Bad

src
β”œβ”€β”€ greetings
β”‚   └── components
β”‚        └── Greeting.jsx
test
β”œβ”€β”€ greetings
β”‚   └── components
β”‚        └── Greeting.spec.js

Data Test Id

In order to uniform data-testid, we decided to use only data-testid. It helps coherency when testing with Testing Library

πŸ‘ Cool

<div
  data-testid="id-of-div-to-test"
/>

❌ Bad :


<div
  data-test-id="incorrect-way-to-use-data-testid"
/>

queryBy ⚑️ toBeDefined

queryBy* methods return an array of Element(s) or null and .toBeDefined() fail only if value is undefined

πŸ‘ Cool

expect(queryByTestId('foo')).toBeInDocument()
expect(queryByTestId('foo')).toBe(null)

❌ Bad :

expect(queryByTestId('foo')).toBeDefined()

See: https://testing-library.com/docs/queries/about/#types-of-queries

Dependencies

πŸ‘‰ Avoid directly calling Material-UI in Cozy Library and Cozy Application

Apart from Cozy-UI, repository should prevent from requiring material-ui as dependency.

πŸ‘‰ Inside Cozy library, any package except cozy-* or react or react-dom can be a dependency

It's complicated to be sure that the application calling a cozy-library has any other dependency. That's why it's recommended to use dependency (when called in the production code)

Example:

  • react-router-dom should be a dependency
  • eslint should be a devDependencies
  • cozy-ui should be a devDependencies and a peerDependencies
See more about those rules

πŸ‘‰ No dependencies between Cozy Library

A Cozy library should not add any other Cozy library. Those Cozy libraries should only be devDependencies + peerDependencies.

Why?

  • in order to reduce library size
  • in order to avoid dependencies cycle

πŸ‘‰ No react or react-dom as dependencies in Cozy Library

A Cozy library should not add React or React-DOM. Those packages should only be devDependencies + peerDependencies.

Why?

  • in order to have different versions of React and compatibility problem with Hooks

We follow the practise of Material-UI or React-Query

Unit Commit

Each dependency upgrade should be inside its own commit, with the possible changes required by the upgrade.

This helps understanding which changes belong to dependency upgrade, and what belongs to the feature of the Pull Request. It is very useful when reverting commits.

Commit messages

A git repository lives with an history that let developers or automatic procedure to find useful information.

They have to look like that:

type(optional scope): Subject

optional body

footer with references to issue tracker IDS

Type

One of:

  • feat: a new feature
  • fix: a bug fix
  • docs: changes to documentation
  • style: formatting, missing semi colons, etc; no code change
  • refactor: refactoring production code; no behavior change
  • test: adding tests, refactoring test; no production code change
  • chore: updating build tasks, package manager configs, etc; no production code change

Scope (optional)

The scope should reflect the part of the codebase, or a specific Component, that is updated by the commit. It should be very concise (one or two words).

Example: feat(Chart): Redraw on data update

Here, the commit is updating the Chart component of the application. We know it directly from the commit message.

Subject

Subjects should:

  • ideally be no greater than 50 characters
  • begin with a capital letter
  • do not end with a period
  • use an imperative tone to describe what a commit does, rather than what it did
  • use the past tense for a fix

πŸ“Œ A note about emojis in commit message

You can use emojis in your commit subject but, if so, you should add it after the type. You can also use emojis in body message anyway you want. Suggested Emoji/task relations

❌  Bad
πŸš‘ fix: when a list contains more than 50 items, the scroll is broken

βœ…  Good
fix: A too long list broke the scrolling πŸš‘

Body aka commit description (optional but recommended)

Pay attention to commit descriptions. It is important that they describe the why of a solution and not only the what and especially the how. This makes it easier to understand architectural choices and decisions that seem clear at the time of implementation but will be much less clear to someone else in 6 months or 1 year.

When writing a body, the blank line between the title and the body is required and you should limit the length of each line to no more than 72 characters.

To summarize:

  • it explains the reason for the change
  • it's searchable
  • it tells a story
  • it makes everyone a little smarter
  • it builds compassion and trust

For more details, see https://dhwthompson.com/2019/my-favourite-git-commit

Footer

The footer is optional and is used to reference issue tracker IDs.

Breaking change

Following the conventional commits, each commit introducing a breaking change must have a BREAKING CHANGE: description. The description should contain a migration path, i.e. a way to overcome the change for the apps using the impacted code.

See commit example here

Example
feat: Summarize changes in around 50 characters or less

More detailed explanatory text, if necessary. Wrap it to about 72
characters or so. In some contexts, the first line is treated as the
subject of the commit and the rest of the text as the body. The
blank line separating the summary from the body is critical (unless
you omit the body entirely); various tools like `log`, `shortlog`
and `rebase` can get confused if you run the two together.

Explain the problem that this commit is solving. Focus on why you
are making this change as opposed to how (the code explains that).
Are there side effects or other unintuitive consequenses of this
change? Here's the place to explain them.

Further paragraphs come after blank lines.

 - Bullet points are okay, too

 - Typically a hyphen or asterisk is used for the bullet, preceded
   by a single space, with blank lines in between, but conventions
   vary here

If you use an issue tracker, put references to them at the bottom,
like this:

Resolves: #123
See also: #456, #789

Pull requests

Before merging a PR, the following things must have been done:

  • Faithful integration of the mockups at all screen sizes
  • Tested on supported browsers, including responsive mode
  • Localized in English and French
  • All changes have test coverage
  • Updated README & CHANGELOG, if necessary

Travis

Deploy

Use the deploy section of travis.yml instead of after_success along with checks on environment variables.

Travis has lots of documentation to easily deploy on npm, github pages, ... you can find documentation here.

If you want use a specific command use script, but avoid environement variable checks in after_success.

❌ Bad :

after_success:
- test $TRAVIS_BRANCH = "master" && test $TRAVIS_REPO_SLUG = "cozy/cozy-realtime" && test $TRAVIS_PULL_REQUEST = "false" && yarn travis-deploy-once "yarn semantic-release"

βœ… Good

deploy:
- provider: script
  skip_cleanup: true
  script: yarn travis-deploy-once "yarn semantic-release"
  on:
    branch: master
    repo: cozy/cozy-realtime

Secrets

Encrypted variables

Travis

All encrypted variables must be set in .travis.yml not in the Travis web interface. They should be preceded by the command to regenerate them. This makes regenerating the variables easy and all the build information is versionned and peer-reviewed.

When using travis encrypt with the --add flag, travis-cli will reformat the entire .travis.yml file and remove all comments. We suggest not using this flag at all.

Example :

# NPM_TOKEN
# To generate a new key: travis encrypt NPM_TOKEN=<token> -r cozy/cozy-realtime
- secure: WDFj9IULpiNSR6h/i8dtmbm+h4hMAUk8EA8wve9sPrJV1GL5qsMgreMYV7uMx7S93K7h1EoILzS1877tLWJJdQ7f7UgakOUVXb41s0GOfQRznDYivqllYE+X9eUkh8gOBjjCF8G3dW4+w4bbY2X97ZC5hhxwQb3DgKWNdOuGLZXZRVmVNLR0XcEkR8p1CKJe4p/iNwianj2L9Q3wk1QvrBP74lwIJIY0i692fW9SKya/BTWGV+9mgGnR8TkAZUViZT2NygNpYxF4NDcXm1Kv2Y47e9Nr9ekGHuzTcCvT/K3hlpxzjo9VgY4lFvjr5izJ/vTScfB0JuHUs3SQFtrz9yI5DBx4OuUm7iJre2dRfUflJhO4KiCtmbZMh7CnBiMSTWFxPHxiD9kZaDU5EunfCRkWcdeSQTwo5bvscHzha7QNUsdzp/xMvOyhqvmoxXapzxymRzRaYntnvkVCZSJIGzHcc9FhsPRd2AQGyk5uffK4lAOVQ+D+d0WCh+5NagEQSPJ6rymsraJpdvR7OBMXVVAmJs76MnNWCQ3DPozIDkNxTxiWWXC02FZBeKrdnVoSLNUCj4jvdLwi4FmQbi2JNMk5zdOojqtt66LiZ8LtjnHzUXZ2dhfRL0URQm97UVagVmWNkte/6PaS/UeHCr193cwthbSFnanjHDclP0eBjvE=

Secrets File

By default secrets file should not been versionned. But if the file will be available on the client side of your application, then we consider it public as said by firebase here (https://firebase.google.com/docs/projects/learn-more#config-files-objects). For instance, file as google-services.json can be versionned in our github repository. If you add this kind of file, please share the word on our security channel.

Feature flags

Master must always be in a state ready to be deployed in production. That is why we use feature flags on each new routes or for each new features not ready to go to production.

To understand better how feature flags works, have a look to our internal wiki.

Release process

Release Schema

You can find more information about the recommended workflow here

Cozy Logo

Cozy Logo

What is Cozy?

Cozy Logo

Cozy is a platform that brings all your web services in the same private space. With it, your web apps and your devices can share data easily, providing you with a new experience. You can install Cozy on your own hardware where no one profiles you.

Community

You can reach the Cozy Community by:

  • Chatting with us on IRC [#cozycloud on Libera.Chat][libera]
  • Posting on our Forum
  • Posting issues on the Github repos
  • Mentioning us on Twitter

cozy-guidelines's People

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

cozy-guidelines's Issues

Process discussion: bug report

I have questions about https://github.com/cozy/cozy-guidelines/blob/master/process/bug-report.md

Once a developer is statisfied with his development he informs the right people

  • Technical Referant of the repository for merging (discussion on the PR, mention the TR in the PR).
  • Product Owner of the repository for closing the issue (discussion is made on the issue, add the label QA to the issue).
  1. Is the fact the PO has to make functional comment within the issue rather than within the PR itself an uneeded overhead?
  2. Do you think the PO shouldn't review a PR from a technical point of view?
  3. If the PO can review a PR from a technical PoV, separating functional comment and technical comment in two separate places seems weird.
  4. What about the PR status. If the PO doesn't do the merge, does he add a tag that say "it's okay to merge that PR for me" like functional ok?
  5. When does the PO close the issue: when the PR is merged or when the version is published?

Improvements

This repo is not much used.

Integration with other docs (explorability, visibility, increase usage)

  • Remove GitHub pages from this repo

  • Add to docs.cozy.io

Usability

  • Add a table of content
  • Reorganize in themes
  • List themes so that it is clear what should and not be in this repo

How we should write mocha tests

Guidelines don't talk much about tests. I think we could add one thing and discuss another one.

  1. We can consider as a guideline that Mocha is the framework to use for server side tests. We can discuss about that, but I think every one will agree.
  2. We should decide on a way to write tests. I think we have a lot of different way to do in our projects and that makes thing messy. If we could agree on how do things (and maybe build a framework on top of mocha to manage it) that could be great.

REST responses

I noticed in controllers we do a lot of that to answer HTTP requests:

# error case
res.send 500, error: true, msg: errorMessage

# success case
res.send success: true, data: "..."

The problem is that the "error" status should be handled through HTTP status code. I suggest we stop doing that and start changing it when we see it in our existing code. Instead, we can do:

# error case
res.send 500, error: errorMessage

# success case
res.send 200, data: "..." # or just data

For the same (standardized) meaning.

What do you think?

Add doc on ducks

Code organization

Actions, reducers, and potential helpers supporting the same functionality should be regrouped in a duck.

Why ? Action and reducers are by their very nature tightly coupled. When separated, refactoring and adding new features leads to editing of several files with tiny changes. With ducks, this is simplified since changes are made in one file instead of several.

Read more : https://medium.freecodecamp.org/scaling-your-redux-app-with-ducks-6115955638be

Standardization of git commit messages

A git repository lives with an history that let developers or automatic procedure to find useful information. Therefore a minimum of standardization is very valuable to every steps in a development/maintenance workflow.

Commits' messages are a part of development and commits' messages guidelines are part of development guidelines.

I found a very welldone commits's messages styleguide: https://udacity.github.io/git-styleguide/

If maintainers agree, I will add a Pull-Request to enhance the current styleguide with some kind of commits' messages styleguide inspired by udacity's one.

"no new line at the end of files"

Can we remove this rule please?

On OSX / Sublime Text the last line is hidden by the scrollbar on mouseover and it's extremely annoying.

Use contextualized localisation files

Since v0.4.0, Polyglot supports nested phrase objects. I suggest we use it to contextualize our localisation files, as it may simplifies future changes to this files. I also recommend (if we stick to my suggestion) to sandbox contexts in separated files, to keep them more consistent.

Here's a little example:

In home, we,ve got many contexts, such as app store, settings, wizards, etc. We could have a file tree like this:

client
    `- app
        `- locales
            |- en
            `- fr
                |- locale.coffee             # the main file to import
                |- app_store.coffee          # app store contextualized phrases
                |- install_wizard.coffee
                `- quicktour_wizard.coffee

Then each context file declares its phrases as usual:

module.exports =
    'hello': "Hello World"

and the main file locale.coffee import them in nested contexts:

module.exports = 
    'home':             "Home"
    'app_store':        require './app_store'
    'install_wizard':   require './install_wizard'
    'quicktour_wizard': require './quicktour_wizard'

Then, we load the main locale with polyglot:

@polyglot = new Polyglot()
@polyglot.extend require "locales/#{@locale}/locale"

and use nested phrases as context:

h1= t('app_store.hello')

fast return

Problem

myFunction = (callback) ->
    doStuff1 (err ) ->
        return callback err if err
        doStuff2 (err, result) ->
            return callback err if err
            callback null, result

vs

myFunction = (callback) ->
    doStuff1 (err ) ->
        if err
            callback err
        else
            doStuff2 (err, result) ->
                if err
                    callback err
                else
                    callback null, result

I prefer and always use the return fast approach. @frankrousseau prefer the if/else approach and we have both ~50% in our codebase

Returns

  • GOOD : smaller / cleanear amha
  • GOOD : more readble amha : this line means if there is an error, pass it above and dont think about it in the rest of the function
  • BAD : easy to forget (coffee-jshint to the rescue)

if/else

  • GOOD : more explicit, expose complexity (force smaller functions)
  • GOOD : easier to expand to handle more error case [1]
  • BAD : amha, need some braintick to see if we handle err, transform it, ignore it
# [1]
doStuff (err, body) ->
    if err and err.code is 'YOLO'
        callback new Error 'yolo'
    else if body.error is 'not_found'
        callback NotFoundError 'that stuff'
    else if err
        callback err
   else
         callback null, body.stuff

How to write polyglot keys?

Linked to #12, we should improve how we write polyglot keys. Actually, we found keys:

  • capitalized
  • lowercased
  • with spaces / dashes / underscores
  • with significant labels or not
  • with structured terms order or not

This lack of consistency make maintenance and future contributions hard to make (even if contextualized phrases will simplifies it). I think we should establish a guideline for those keys. I recommend:

  • lowercase only
  • use underscore, no spaces allowed (more below)
  • have structured order inside context, that name component, state, and element (i.e. btn_validate_settings)
  • keep them sorted (a>z) so they can be found quickly and prevent overriding

Why use underscore?

IMHO, underscore should be preferable because:

  • polyglot recommend and use them in their doc
  • spaces are not very well-handled in JSON notation
  • keys are… well… keys ; they're just ids, and add spaces may be confusing
  • it's simpler to generate an underscored key from a name or a slug (which itself can easily be a key πŸ˜‰)

Emojis in commit message

Some of us like to add (related) emojis to their commit messages, some of us don't.

What do we do?

Can we use emojis? And if so, do we set a rule to its location in the message? In the beginning just before the type or at the end of the message?

Discuss or vote!

πŸŽ‰ At the beginning
❀️ At the end
πŸ‘Ž Against emojis
πŸ˜• I don't care

Add an optional scope to git messages

I think adding a scope is very informative in some commit messages. To use an example from our current guidelines, I think this commit message:

fix(contacts): A too long list breaks the scrolling

is way more informative than the version without the scope:

fix: A too long list breaks the scrolling

The fact that this refers to the contacts list can be written in the body, but I think that's not the place for such an information.

Also, this is a good help to write changelogs (manually or automatically). semantic-release uses it, for example.

Obviously, sometimes this scope doesn't add any information, so it must be optionnal.

See https://conventionalcommits.org/#spec-item-4 for more informations about the scope in the conventional commits specification, which widely inspired udacity.

What do you think about it ?

Add contributing guidelines

Explain some good practices for making a PR:

  • don't commit build files;
  • make the PR on development branch…

Once done, we should update the README in every project.

Fonts

By @frankrousseau (in french):

Pour les fonts l'idΓ©al serait d'avoir partout :

  • maven-pro-light-200
  • source-sans-pro

Release cycle / process

I thought about the suggested release cycle yesterday, and wondered how to actually mark something as "released". Currently it's supposed to be on the last day of a sprint, the TR tags a new version.

What about the app's build? Do we build in each PR or do we wait for the release to actually build it?

Pros:

  • more accurate distinction between release and not release feature (= when users installs an app, they install upstream/master so they might end up with unreleased stuff, potentially less stable. Users can't say "I only want to have stable versions")

Cons:

  • longer feedback cycle (= no simulation of a staging env if we update the app ourselves before the release to try them in real life)

While I think the "I only want to have stable versions" could be managed by the Home itself, and I believe the cons are bigger than the pros, I wanted to bring up my thoughts on the matter to the team.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.