Style Guide

Overview

With some tolerance for deviation, this project adheres to Google's TypeScript style guide. https://google.github.io/styleguide/tsguide.html

This document serves an extension of that guide, and to call out some best practices relevant to this project.

ES Lint

Linting rules are based off of the ES and TS recommended rules with some opinionated adjustments. This section attempts to justify these preferences.

no-explicit-any: off

The intention of this rule is important; public APIs using type any should generally be narrowed. However, there are too many cases for exception. Unit tests, conditional types, and type boundaries as examples often use type any for legitimate reasons.

no-non-null-assertion: 'off'

Non-null assertion (e.g. foo!.bar) should only be used in cases where the non-null assertion is safe, and the control-flow inference cannot otherwise determine its safety. This is disabled because at the time of writing the inference misses too many common cases. For example, consider this safe expression:

const x = foo?.bar == undefined ? 1 : foo!.bar

This would result in a warning. Thus, either requiring a verbose // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, or using the null coalescing operator which would both incorrectly type x, and add slight runtime overhead.

no-empty-function: 'off'

Empty functions are a common practice when implementing interfaces but have no operations.

To run eslint fixes, in the terminal run: eslint . --fix

Prettier

In order to prevent bike-shedding, prettier is used for code formatting rules. The separation of linting and formatting rules is recommended for several reasons. First, ESLint's linting rules are executed one at a time, making them considerably slower than formatting rules. Secondly, showing IDE errors for formatting rules creates noise that distracts from development. Finally, all formatting rules have automatic fixes.

To run prettier fixes, in the terminal run: prettier . --write

To run both prettier and eslint auto-fix commands, run: bb lint:write

Best Practices

Idempotence should be preferred where possible. Example set({ id, foo, bar }) is preferred over create({ foo, bar }) and play()/pause() is preferred over togglePlay()

Inputs should be read-only where possible unless explicitly indicated as an output variable.

Having mutable inout variables should be avoided except in cases where allocation would cause a significant performance bottleneck. For example, within a long-running loop that executes many times per second.

Property getters should never have side effects. For example if get next() advances a cursor, it should be changed to the method next(). Property getters may have lazy computation, but the results should be cached and repeat calls to getters should always be considered safe.

Module Naming

• Use lowerCamelCase
• Use the plural for packages with homogeneous contents and the singular for modules with heterogeneous contents.

File Naming

• Use lowerCamelCase (This differs from the Google TS Style Guide which uses lower_snake_case) unless the file represents a single class, in which case it must be UpperCamelCase.

Property Naming

Follow TS Guide - Naming Style for property naming, with these additions:

Private properties that shadow a public or protected getter with the same name should be prefixed with an underscore.

Booleans should not be prefixed with is. Example: Prefer active over isActive.

Acronyms should use camel case. Example: Prefer HlsParser over HLSParser.

type or interface?

Understand the differences between type and interface, this is documented: at https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#differences-between-type-aliases-and-interfaces

Prefer type when using:

• Union & Intersection Types
  • type Status = 'success' | 'error' | 'loading'
• Aliasing Primitive Types & Functions
  • type Callback = (data: string) => void
• Tuple & Mapped Types
  • type Point = [number, number]
• Working with Conditional Types & Utility Types
  • type IsString<T> = T extends string ? 'yes' : 'no'

All other cases prefer interface, it is better performance for the compiler, better compiler error messaging, provides IDE support for navigating to base type, and supports declaration merging.

index.ts files

index.ts files are auto-generated by running the script indexFiles:write. index.ts files serve as entry points for build entries and index.d.ts declaration files.

Workflow Rules

Development environment is a personal choice. While it's acceptable to commit IDE files that make setup easier for popular IDE choices (namely IDEA or VSCode), any rules such as formatting, testing, compiling, and serving the examples should be able to be done via the command line and repeatable within CI.

Tests should be able to be run iteratively and as rapidly as possible.

Time

Time, when expressing a timestamp (ms from Unix Epoch), use the Timestamp number type alias, or a Date object if Date utilities are needed. When the date should be read only, the ReadonlyDate type alias should be used.

Time deltas are expected to be in seconds unless otherwise noted with a suffix. For example:

• time: number would be expected to represent seconds
• timeMs: number milliseconds
• timeUs: number microseconds
• timeNs: number nanoseconds

Regexp

Regular expressions may be used as constants, however, be mindful of a stateful RegExp. Sticky or global RegExp objects should be considered stateful. When using a stateful RegEx, the RegExp must be reset to its initial state at the end of its use, and there must not be any possibility of its concurrent use within a function.

Spelling

Spellings should be American English.

Testing

Contrary to Jasmine's suggestion in the FAQ, arrow functions are used in it, beforeEach, and afterEach. Tests are written in TypeScript, and avoid the practice of having a dynamically typed context. Without a context, arrow functions are slightly more terse and therefore preferred.

When naming specs, describe should be named with the condition and may be nested, it specs should describe what is expected. e.g.

describe('when enableFlacOpus is true', () => {

  beforeEach(() => {
      // set enableFlacOpus to true
  })

  describe('and streamingQuality is set to HD', () => {

    beforeEach(() => {
      // set streamingQuality to HD
    })

    it('plays Flac tracks when the device supports Flac and Opus', async () => {
      // add expectations
    })
  })
})

Jasmine Spy Imports

Always import jasmine spy functions using TypeScript import alias syntax instead of accessing them directly from the jasmine namespace:

✅ Correct

import createSpy = jasmine.createSpy

// Usage
const mockFunction = createSpy('mockFunction').and.returnValue(true)

❌ Incorrect

// Don't access directly from jasmine namespace
const mockFunction = jasmine.createSpy('mockFunction').and.returnValue(true)

Test Imports

Public API Usage

Tests should import from the public API (@amzn/vinyl) rather than internal paths (@/):

✅ Correct

import { createResourceTypeFilter, type MediaQualityFilter } from '@amzn/vinyl'

❌ Incorrect

import { createResourceTypeFilter } from '@/track/filters/resourceTypeFilter'
import type { MediaQualityFilter } from '@/track/filters/MediaQualityFilter'

Common Jasmine Import Aliases

import createSpy = jasmine.createSpy
import createSpyObj = jasmine.createSpyObj
import any = jasmine.any
import objectContaining = jasmine.objectContaining

Git Commits

Commits must follow conventional commit standards.

Use git cz to start commitizen wizard.