Skip to content

Contributor Guide

Christina Ahrens Roberts edited this page Mar 26, 2024 · 45 revisions

Table of Contents

TL;DR

You might start by checking out this welcome slide show and the other contents of this Google Drive folder.

Libraries and Technologies

Our Coding Philosophy

We would like our contributors to understand the principles behind our code base. With this information we hope to make it easier for you to contribute and understand why we have made certain decisions in our code. As you read through the document you should see these philosophies emerge in our coding style and practices.

  1. Write simple code: clear, concise and easy to understand

    • Simple does not mean easy, solutions to difficult problems can be coded with simplicity
    • This is difficult to do in practice, but we always strive for simple code
    • Beware of clever solutions that are hard to understand
  2. Ask: why / what is the problem we are addressing?

    • Does it bring value to the user?
    • More features is not always better
    • Just because we can do something, does that mean we should? Is it the right thing to do?
    • Are we solving an issue for the user, or just completing a prescribed task?
    • Making sure we understand the problem as a whole, pushing back and getting clarification when it is not clearly understood
  3. Strike a balance between following standard patterns and thinking outside the box

    • Is something hard to understand? Can it be done better? Do it! This keeps tech debt down, the code base simple, easy to understand and work with.
    • Standards can be a great place to start and may be the best answer
    • Accepted or industry standard approaches have value and get us close to where we want to be, rethinking problems from first principles can spur innovation and deliver better results
    • The world around us is constantly changing and new solutions can present themselves when rethinking a problem
    • We accept this within our own codebase as well. If the rationale for doing something no longer holds, be willing to change it. "That is how we have always done it" is not a good rationale
  4. Be willing to give and receive feedback

    • Being able to give and receive feedback is an excellent way to improve your skills as an individual and support the project
  5. Usability and Accessibility are a high priority

    • Ensure our application is accessible so everyone can use it
    • Accessible applications tend to be more usable for all
    • Accessible pages are easier to test
    • Test how the users will interact with the application
  6. Mentoring is an integral part of becoming a better engineer

    • The ability to explain and answer questions demonstrates greater understanding and exposes areas where you can improve
    • Ask questions! This helps you and the person explaining the answer solidify knowledge and exposes areas of confusion.
      • Don't be afraid to ask the same question multiple times, that is part of learning
  7. Keep the development environment simple so the developer experience is painless

    • Use tools that keep the environment simple and straightforward to use
    • Provide clear error messages where appropriate

Ongoing Efforts

  • A number of ongoing efforts are active to make code structure and related improvements to Terra UI. Developers should be aware of these efforts since a number of them may influence specific guidance and recommended code pattern migrations. As some of these efforts mature, this guide and related wiki content will be updated accordingly.
    • Typescript - Terra UI is now in "Open Adoption" phase of Typescript. Teams are free to openly and aggressively adopt Typescript in their code areas, and are advised to communicate on terra-ui slack channel on desired TS conversions/improvements in shared code areas.
      • This TS tips/guide doc is WIP.
    • Prettier/TS-Lint - Code auto-formatting and gradual alignment with airbnb style guide is ongoing.
    • JSX/TSX Adoption - visual code using react-hyperscript-helpers is slowly being migrated to more standard TSX. This is a desired, but low-priority migration that should be done AFTER Typescript conversion of a file, and 80% or better unit test coverage is achieved.
    • lodash/fp migration - lodash/fp will be replaced with a modern Typescript-first fp library
      • We will be vetting Remeda as our top candidate.
      • We will stop using _.curry and instead align with data-first, data-last, or both forms in Remeda for custom HoF's
      • We will continue to use flow() for folks that like the pattern, but must have strong type-safety.
      • doc: Lodash/FP Future
    • Cross-Team UI Code Sharing - Terra UI is serving as the anchor project to implement DSP-wide cross team ui code sharing

Coding Guidelines

General

  • Use TSX instead of react-hyperscript-helpers.
    • We are moving away from react-hyperscript-helpers and encourage new code to be written in TSX.
  • Prefer const over var or let
    • Re-assignable variables introduce additional state and potentially side effects into a function. This can make it difficult to reason about the code and increases the likelihood of bugs.
  • Use !! when a non-boolean constant is used in a logical operation
    • e.g. !!myObjects && doSomethingWith(myObjects))
    • Note that !!0 === false evaluates to true, so this is not a workaround for JavaScript's poor decisions around falsy values.
    • condition && element is a pattern frequently used to conditionally render elements in React. 0is falsy, but if condition is equal to 0, then React will render a 0 as opposed to rendering nothing if condition is false, null, or undefined. Coercing to a boolean avoids this.
  • Only use ? (optional chaining) if the object you are accessing a property or function from could legitimately be null or undefined, and your code is robust to the returned value of the optional chaining being undefined. Optional chaining prevents an exception from being thrown, but it should not be used indiscriminately.
  • While encoding URLs for network requests, use qs.stringify to properly format query strings: i.e. qs.stringify({ thing1: 'hello', thing2: variable }) will give you thing1=hello&thing2=${variable}, but will take care of any escaping for you.
    • Note that the question mark ? is not included. The browser considers http://localhost/ and http://localhost/? to be different URLs.
  • Consider destructuring values down to the most atomic property in most cases (e.g. const { prop1, prop2 } = someObj).
    • Pulling out only what you need rather than taking the entire object even when you are not using it, makes code easier to reason about.
    • It also allows for defining default values and aliases in one place rather than scattering them throughout call sites.
    • Sometimes pulling out too many properties and/or nested properties can be cumbersome and make code more difficult to read. Therefore we are not recommending using this technique in every case.
  • Conditional logic adds complexity, so use it judiciously.
    • Use reasonable default values when appropriate rather than using imperative code or conditionals.
    • We use short circuit evaluation when the entire expression can fit into a single line.

Naming Conventions

  • Use verbs for methods and nouns for classes and constants.
  • Use camelCase (not UPPER_CASE) variable names, including for those whose values never change.
    • Most variables should be declared with const, so there is no reason to give them special casing treatment.
    • For components and classes, we capitalize the initial (a.k.a. TitleCase).
  • For folder names, use kebab-case.
    • Kebab-case is preferred when constraints allow. snake_case readability suffers whenever it is displayed with a border or underline.
  • For strings, default to using the case from the API unless it’s only for display.
    • The goal is to preserve the data’s fundamental structure as much as possible from get => render => post.
  • Import as namespace when it's a lib of things with generic or context-specific names (e.g. import * as Utils from 'src/libs/utils').
    • Search for import * as in the codebase to see more examples.
    • This practice helps with namespacing.

Images

The steps below can substantially reduce the size of an image, which can improve usability in many cases, especially on slower network connections:

  • When checking in svg files to images folder, first run yarn optimize-image-svgs on them.
  • When checking in image files, first drag and drop them on the website squoosh or tinypng, and then check in the optimized versions.

Lodash

  • We prefer Lodash FP functions over native functions for the following reasons:
    • Lodash FP functions have auto-currying built in. This is especially useful for composability. If someone wants to add functionality, it makes it easy to do something like add the function to a flow.
    • Lodash FP will not mutate an array. In the case of native slice, it also returns a new array. Beware there are other functions that can mutate (e.g. push/pop).
    • Mixing native and Lodash functions can lead to confusion about behavior.
  • Use _.get() for its auto-currying, or when describing a path in an object that contains multiple variables, and otherwise use optional chaining.

React

  • Use function instead of class components.
    • Function components better support our functional coding style.
    • They use React hooks instead of lifecycle methods in a function component.
      • If you need more custom behavior, look at the code base for our custom hooks.
      • Custom hooks will be prefixed with use (e.g. useOnMount).
  • Reduce the amount of state in your functions and components - what is the minimum state needed?
    • Don't save derived state: retain a single source of truth.
  • Use CSS-native functions (e.g. calc) only when Javascript functions aren’t sufficient (e.g. when one needs to use a relative value like ‘rem’ as part of a calculation).
    • This helps to limit the expertise required to be productive in the codebase.

Styling

  • Style components directly using the style prop.
    • This keeps the styling close to the code allowing developers to easily add styles without having to switch contexts to CSS or another file.
    • We can take advantage of all existing tools and patterns to enable partitioning and sharing of styles, since they do not require special treatment.
    • This also prevents developers from having to deal with CSS classes and side effects of the cascade.

Coding Practices

  1. Write unit-testable code

  2. Keeping code simple is easier said than done.

    • Some general thoughts on simplicity to take into account when contributing code

      • Could someone else explain your code?
      • In a year from now will you still understand this code?
      • Did you have to use comments to clarify your code, or is it self explanatory?
      • Do your functions and components need to know implementation details of other functions and components?
      • Is the data decoupled sufficiently from the logic?
    • Concepts that can make your code simpler

      • Ensure functions and components are single purpose
      • Avoid side effects
      • When you need to deal with side effects, use the right abstractions to make it clear we are dealing with a side effect (e.g. useEffect)
      • Make use of small, simple helper functions
      • Keep functions pure
      • Utilize our existing constructs (e.g. withErrorHandling instead of try/catch)
    • Concepts that will add complexity to your code

      • Adding a lot of state
      • Sharing state across components
      • Excessive branching / conditionals
      • Adding special cases
      • Using abstractions that are too powerful for the intended result
        • e.g. using a sledgehammer to hang a picture
      • Premature optimizations
  3. Favor functional, declarative coding

    • Functional code that is side-effect free or has predictable effects is easier to reason about and test
    • This helps us keeping data separate from the application logic
    • We prefer composable constructs such as wrappers (e.g. withErrorHandling) as opposed to imperative code
    • When the need for imperative code arises, make sure the coding style is clearly imperative (use if/else rather than ternary expressions)
    • Take advantage of currying where you can
  4. Understand the problem

    • Before writing code, it is good to understand what value the change is bringing to the user
    • It is also good to understand the user flow - how the user uses the application - to make sure we are providing the correct functionality rather than just prescribed functionality
      • For example, a user may request making a data column wider so they can more easily copy and paste data
      • Diving deeper and asking why they want to copy data in the first place may yield more information, unveiling what the user actually wants vs. what they asked for
  5. Feedback

    • We prefer slowing down and iterating several times on a PR to ensure the code is right and in line with our standards rather than quickly pushing code through
    • We emphasize a high level of code quality in our codebase to prevent technical debt and keep the application easy to develop
    • We encourage comments and discussion in PR's to release high quality, understandable code
    • To merge the code with our dev branch, at least 1 PR approval from a member of the Saturn team is needed
  6. Do not be afraid to change existing code to make it easier to work with

    • If you find yourself trying to work around existing code think about what improvements could be made to it rather then continuing on the workaround
    • Feel free to reach out to any member of the team if you are unsure of how to proceed before investing too much time into a solution - we love to help our contributors!
  7. Release often

    • Once code is merged to dev, it is considered production ready and deployable
    • We release every weekday. Once you have merged your code it will be live in production around 11:00AM
    • A frequent release cycle helps us to keep our deployment process simple which allows us to rapidly fix issues as they arise

Accessibility

When writing and reviewing PRs, remember to check your interface for accessibility. Here is a list of common issues that you should check (note that this is certainly not an exhaustive list):

  1. Color contrast meets WCAG standards
  2. Color is not the sole indication of state or information
  3. All input elements have labels
  4. All interactive elements are keyboard focusable with a logical tab order (try interacting with your interface without the mouse!)
  5. Your HTML is semantic or has the proper aria roles defined
  6. All images have the alt attribute present (if image is only decorative, use empty string for value)
  7. Headings are used logically in descending order (H1, H2, H3, etc.)

There are several static analysis tools that you can use to catch many of the common issues. Note that axe is enabled by default in development mode, so if you open the JavaScript console while running Terra locally you will see accessibility errors/warnings as you navigate Terra. Here are links to other options/plugins that can be run on demand in any environment:

  1. axe Dev Tools
  2. ANDI
  3. Lighthouse, built into Chrome dev tools

If you have the time, try using VoiceOver (on Mac) to check if your page is understandable and usable with a screen reader. Screen reader resources:

  1. Screen reader Questions and Answers
  2. Using VoiceOver

General Accessibility Training/Resources:

  1. W3C Introduction to Web Accessibility Course
  2. WebAIM Introduction to Accessibility
  3. Accessibility Overview by Mark Sadecki
  4. WCAG 2 Standards
  5. Tutorial that covers basic concepts with simple exercises

Automated testing options (automated testing can catch about half of all accessibility issues):

  1. Integration tests can call verifyAccessibility to check for accessibility errors anywhere on the current page. This will verify color contrast of enabled items, among catching other issues that can be statically detected.
  2. Unit tests that render components can use toHaveNoViolations to look for accessibility issues in the rendered component. Note that color contrast cannot be verified with this type of test, as the component is rendered in a virtual DOM.

Visual Design Library and Notes

We are using Chromatic to publish our Storybook library. We are still in the early days of writing stories, and certainly encourage contributions in this area!

Notes: this section is a work in progress. We will add notes on visual design preferences as questions come up from contributors.

  1. Minimum width
    • The application has a minimum width defined in the style.css file. This width was determined from the real world screen sizes we were seeing from users of the app.

Other Resources

Simple Made Easy - Rich Hickey

Clone this wiki locally