Skip to content

Coding Guidelines

Jump to bottom
Nelson Leite edited this page Mar 1, 2023 · 14 revisions

(WORK IN PROGRESS)

The following code guidelines should be followed by contributors to this package.

Please do not file issues about these guidelines.

Exporting modules

  • Make sure all public types and modules are exported and accessible through the root index file of the package they are a part of (the index test snapshot file of each package is used to test for that).

    • If a type is being used in the definition of another type that is public and is being exported, export it as well.

  • Make sure no privately used modules end up being exported from the package.

  • If a type defined in a @types package is used in a publicly exported type, add the @types package as a dependency of the package (not in the monorepo!). Otherwise, the @types package can be installed in the monorepo as a devDependency.

  • When defining a default export for a function by using an arrow function, do not export from the same line like this:

    export default (param: string) => ...

    but instead export it like this:

    const myFunc = (param: string) => ...

export default myFunc;

    or even better use a function expression with a name:

    export default function myFunc(param: string) => ...;

  • Action types, reducers, middlewares and entitiesMappers should be exported under a name that refers to the area. For example:

    • bags action types are exported as bagsActionTypes;

    • bags reducer is exported as bagsReducer;

    • bags entities mappers are exported as bagsEntitiesMapper;

    • bags middlewares are exported as bagsMiddlewares;

Importing modules

  • Use relative imports to import modules living in the same package instead of self-referencing the package.

  • When using relative imports, prefer imports directly to the file instead of importing from the index when possible.

    • This is to avoid having files that import other files unnecessarily and might lead to circular dependencies. Importing from indexes is most problematic when they export a lot of different modules (i.e. the root index files). They are fine if used sensibly.

Types

  • All code files (i.e. files inside the src folder) should be in typescript.

  • Prefer using type to define new types instead of using interface as they are more flexible.

    • One drawback of using type is that when using type intersections for extending a type, conflicts might not give errors. For example if a type defines a property x that is a number and a type that extends it defines a property x that is a string, the intersected type will have the type for the x property calculated as never while if an interface was used, an error would be raised. To deal with this, when defining the intersection use Omit type to remove the property and then redefine the property with the new type.

    • One drawback of using interface is that it allows declaration merging, i.e., the interface can be extended by adding other declarations with the same name. This can lead to some very very nasty problems so you should avoid it most of the time. When in doubt and possible, use type instead.

  • Make sure the types are defined and exported in the correct package. For example, a type that is used solely in the @farfetch/blackout-redux package should not be defined and exported in the @farfetch/blackout-client package.

  • Avoid abusing any. There are times when they are necessary but most of the time we can use a more specific type. If there is not a more specific type and unknown can be used, prefer unknown to any.

  • Make sure the types of selectors created by createSelector are inferred correctly as most of the time they are not. If not, then declare the correct type of the selector including the return type of the function.

  • Return types should not be defined most of the time in favour of having typescript infer the type. Exceptions should be made when the function is not trivial to make sure that no value of an incorrect type is being returned.

  • If you need to create a type for a value that is supposed to conform to a public standard (example: JSON Schema) check if there is a @types package available that you can use instead of defining your own types.

  • When possible refer to other types instead of hardcoding a type. For example, if you are typing a function that receives the user id as the first parameter which is a number, do not type the function like this:

    function doSomethingWithUser(userId: number);

    but define it like this, instead:

    function doSomethingWithUser(userId: User['id']);

  • Do not redefine a type multiple times if they are the same and are being used by different modules. Follow these guidelines:

    • Find if the type you are creating already exists in the package you need, for example, by searching for types that contain some of the properties of the new type.

    • If there is another type that matches exactly, check if it makes sense to have a more generic name in order to be shared by the modules. For example, if there is an Environment type that can be reused by many modules, use it instead of defining a specific type or a different alias for that type like <Area>Environment (CheckoutEnvironment for example).

    • If there is another type that matches exactly and it makes sense to be treated as different types, then create an alias for that type instead of redefining the type. For example: BagOperationChange and CheckoutOrderOperationChange are aliases to the same base private type OperationChange.

  • Use @ts-expect-error, @ts-nocheck and @ts-ignore directives sparingly.

    • There are some valid use cases (like in testing using an invalid value for testing errors) for them but most of the time they should not be used. When they are needed, provide a description in front of them.

  • The action parameter in redux reducers should be typed as AnyAction and not as the expected action type since reducers can be called with all actions.

Naming

  • Keep consistency with the names used throughout the code.

    • Especially for names that are part of the public API like in clients, actions, selectors, types...

  • Do not use generic names for public modules/types unless they represent something that is not specific to a feature. For example, use names like BagOperation and CheckoutOrderOperation instead of just Operation as those names are more meaningful and only used by those areas.

  • Enumerations should use PascalCase naming convention for both the name of the enum and the enum members. For example avoid this definition:

    enum MY_ENUM {
        VALUE_1,
        VALUE_2,
        VALUE_3,
    }

but use this definition, instead:

    enum MyEnum {
        Value1,
        Value2,
        Value3,
    }
  • selectors that return the loading state related to a request that will load a group of entities (for example, from a request to fetchUserOrders, fetchUserAddresses, etc...) should be named as are<Entity>Loading (for the UserOrders example, its loading selector should be named areUserOrdersLoading and not isUserOrdersLoading).

File names

  • Make sure file names are correct (without any lingering spaces or misspelled names).

  • Make sure the modules are defined in the correct files:

    • types should be defined in a .types.ts file

      • Exceptions could be made for types that are used in test files or that are very simple and private which can be defined directly in the file it is being used.

        • Mostly for temporary types used internally.

    • Fixtures should be defined in files that end in .fixtures.ts inside folders named __fixtures__.

    • Test files should be defined in files that end in .test.ts inside folders named __tests__.

    • When in doubt, look for similar implementations in other areas for guidance.

Redux

  • Selectors that return values that are normalized in the store, should return them denormalized when possible.

    • Take special consideration to use createSelector to return a new object only when the entity or its sub-entities have indeed changed.

      • Red flags for createSelector:

        • Do not use an input selector that returns the whole state of the app, as the selector will not memoize correctly since the state can change when any value in the state has changed, discarding the memoization unnecessarily.

        • Avoid calling a selector that is defined with createSelector inside another selector with different arguments as it will prevent the correct memoization from taking place. For example, the selector to return a wishlist should not call the selector to get a wishlist item for each item of the wishlist if the get wishlist item selector is defined with createSelector.

  • Not all requests might need to be stored in the redux store. If a request is likely to not be performed by different components that are shown at the same time, you can avoid saving the state in redux but you will need to provide a hook that manages the state of the request internally so that consumers can obtain that state.

    • Examples of requests that do not need to be in the store: Requests for data that is ephemeral like "return pickup requests" or "order available activities"

    • Examples of requests that can be in the store: These are requests for data that might persist longer in a user session, like "bag", "wishlist", "orders" or "user".

  • State for requests that accept a query parameter:

    • For requests that might accept a query parameter, think if it might be necessary for the consumer to have the state segregated by query instead of having only a single value for the last request performed. If you decide to have the state segregated by query, the selectors will need to receive the query as a parameter as well and a hashing function should be used internally by these selectors and by the action to get the hash that will be used to find the correct state in the reducer. Do not force the consumers to generate the hash themselves unless there is a strong reason to do so.

  • Consider if reset state functions should be provided for the consumer to reset the state of some requests.

    • If provided, they should reset both the state and if applicable, the entities.

  • Selectors that do not return a computed value, i.e., return the response from the endpoint should return undefined when there is no data fetched from the server. This is to help consumers distinguish when there is a need to request data from the server or not.

  • Reducers that deal with state that is relative to the current user (orders, addresses, bag, wishlist, etc...) should be reset by listening to the LOGOUT_SUCCESS action.

    • For entities an entitiesMapper for the LOGOUT_SUCCESS action should be provided as well.

  • Action objects:

    • Meta parameters of actions should be objects containing the meta parameters passed on the meta property of the action.

    • Payload of actions should be passed on the payload property of the action and can be other things than an object if it makes sense for the action (For example, the action can contain an array on the payload property).

  • Entities

    • To decide if an entity should be created or not, evaluate if it can be shared (now or in the future) with other entities. If not, do not create an entity and place the result inside its state slice directly.

    • Always use the normalizr package for entities generation with a dedicated schema.

Dependencies

  • Make sure that the runtime dependencies of a package are defined inside the dependencies section of the respective package.json of the package and not in the package.json of the monorepo.

    • An exception must be made for packages that might be used by the applications themselves (like react) which should be specified as peerDependencies of the package instead of a dependency to avoid having duplicated packages installed in the node_modules of the consuming application.

  • The monorepo should only contain devDependencies which are used for management tasks of the repo (jest for testing, lerna for publishing, etc...).

    • Careful with packages that are installed in devDependencies of the monorepo but are also necessary at runtime for a specific package. They might not fail when running tests since it will use the hoisted package of the monorepo but will fail at runtime when installed in another app that does not contain the required package.

Tsdoc

  • Make sure you describe the module functionality concisely and highlight any problems / gotchas that it might take. For example, if there is a client to retrieve orders for a registered user and another client to retrieve orders for a guest user, highlight that fact on the description and point the user to the other client if that was his intention. Something like this (definition of the client that fetches guest orders only): "Fetches orders for guest users. If you want to fetch the orders for registered users use the 'XXX' client, instead."

  • When documenting functions, make sure its description, and the descriptions of its @param and @returns directives match the definition of the function.

    • @returns do not need a - before its description but @param does.

  • Make sure the format satisfies the tsdoc format and no jsdoc tags which should not be used are inserted (like definition of the type in a @param directive as tsdoc will infer the type from typescript).

Errors

  • Make sure that the errors thrown by clients and action factories are of the type BlackoutError. There is a toBlackoutError function in the client package that takes care of converting an Error instance to a BlackoutError if necessary.

  • Do not throw values that are not errors like throw 'error message' instead of throw new Error('error message')

Hooks

  • Do not ignore or disable the warnings generated by the exhaustive-deps eslint rule. Analyze if the useEffect is really necessary or can be refactored in a way that all dependencies are used inside the effect.

  • If you need to create a custom hook, follow the conventions used in other custom hooks provided by the react package:

    • If the hook can automatically fetch data, accept an options parameter which contains an option called enableAutoFetch to control this auto fetching behaviour.

    • Data should be returned in the data property unless the hook can provide multiple data from different actions. Returned data should have the same reference unless it has changed (use useMemo for that). If there is no data to return, return undefined. Avoid returning objects that are partially filled with properties from different endpoint calls when possible.

    • Actions should be returned under the actions property.

    • Data and actions returned should be related to the context of the hook call. For example, if you are creating a hook called useOrder which accepts an orderId parameter, the data returned should be from the order whose id is equal to the orderId parameter received. Actions that use the orderId parameter, should omit it from the consumer and use the orderId inside.

      • There are some cases where you might want to allow the user to override the implicit parameter (in this example, the orderId) of the action. If you need to support this case, declare the parameter as optional and it should be the last parameter of the function. This is intentional to make the most common usage of not overriding this implicit parameter less cumbersome. But remember that the returned output of the hook should continue to respect the parameter passed on the hook and not the override.

    • If you can automatically reset data when the parameters change on a subsequent hook call, do it. However, do not do it in a useEffect but instead do it directly in the hook's body. Check useCheckout hook for an example when the passed checkoutOrderId is different than the one that is stored in redux data.

Clone this wiki locally