xo-stack/CONTRIBUTING.md

# Guidelines

## MR/Commit Messages

NOTE: Not necessary to follow this until V1.0 release.

Use a [Conventional Commit](https://www.conventionalcommits.org/en/v1.0.0/) format when creating MR's. For example:

```
feat(Primitives): Add Transaction Primitive
```

... or to indicate a breaking change (using `!`):

```
chore!: Drop support for Node 16
```

This will ensure that we can automatically generate correct [Semantic Versions](https://semver.org/) and changelogs between releases (using Lerna).

A table of common prefixes (conventions) is below:

| Change   | Description                                                                                                 |
|----------|-------------------------------------------------------------------------------------------------------------|
| feat     | A new feature                                                                                               |
| fix      | A bug fix                                                                                                   |
| docs     | Documentation only changes                                                                                  |
| style    | Changes that do not affect the meaning of the code (formatting)                                             |
| refactor | A code change that neither fixes a bug nor adds a feature                                                   |
| perf     | A code change that improves performance                                                                     |
| test     | Adding missing tests or correct existing tests                                                              |
| build    | Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)         |
| ci       | Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs) |
| chore    | Other changes that don't modify src or test files                                                           |
| revert   | Reverts a previous commit                                                                                   |


## Coding Guidelines

These guidelines represent best practices. While exceptions may occasionally be necessary as "the lesser evil," strive to adhere to these principles whenever possible.

1. Try to minimize state

   State is a complexity multiplier. Keep mutable class members to an absolute minimum, as each stateful property exponentially increases cognitive load and potential for bugs. When designing classes or components, first attempt a stateless approach, adding mutable properties only when demonstrably necessary.

2. Try to avoid `any` types

   Use `unknown` instead, as it provides better type safety while maintaining flexibility when needed.

3. Prefer immutable patterns

   Use immutable data structures where possible to improve code predictability and testability.

   For example:

   ```ts
   // GOOD:
   const mappedData = originalData.map((item) => item.field);

   // AVOID:
   const mappedData = [];

   for(let i = 0; i < originalData.length; i++) {
     mappedData.push(originalData[i].field);
   }
   ```

   Do note that there are some cases where it may make more sense to mutate data inline (e.g. as an optimization for very large data structures).

   Use your best judgement, but treat immutability as the general rule.

4. Avoid Typescript, Linter and Formatter Directives

   These exceptional cases should be minimized as they:
   - Create inconsistency in code standards
   - Make maintenance more difficult
   - Often mask underlying issues that should be properly resolved
   - Can lead to unintended side-effects throughout the codebase

5. Avoid additional dependencies.

   Try to avoid pulling in additional packages. Not only can this bloat the project, but can also cause compatibility issues between environments (Web, Node, ESM, etc). We want `@xocash/stack` to "just work" without devs having to tailor build configurations.

   Obviously this is an ideal: Sometimes it is simply not feasible or sensible to rewrite complex functionality that can be better done by a well-built package. However, please try to adhere to the following:

   - Ensure the package is of high quality (well-written, well-tested).
   - Does not drag in additional dependencies itself (note that `devDependencies` are fine. These will not be installed when leveraged from our packages).
   - If the package being used **is heavy** and we have no other choice but to use it, consider whether we should package it separately to avoid problems. For example, if we build a `BlockchainBCHN` adapter, we will likely want to package this on its own under `@xocash/blockchain-bchn` as it only works in NodeJS environments.

6. DO NOT use Typescript "enums".

   TypeScript enums have several issues that make them problematic:

   - They generate unnecessary runtime code and don't tree-shake well, increasing bundle size
   - Numeric enums allow unsafe reverse lookups and can be assigned any number value
   - They don't work well with module augmentation and can cause circular dependency issues
   - They're not compatible with Node.js's --experimental-strip-types feature, which only removes type annotations and doesn't transform enum syntax

   Recommended alternatives: Use const objects with `as const` assertions or string literal union types instead, which work seamlessly with Node's type stripping.

7. "When in Rome..."

   For conventions not explicitly mentioned, follow the "when in Rome" principle. Try to identify and use the patterns and conventions that are already established elsewhere in the codebase.

## Patterns

### Dependency Injection

We use Dependency Injection (DI) for several key reasons:

- Improved testability: Dependencies can be easily mocked during unit tests
- Reduced coupling: Classes depend on abstractions rather than concrete implementations
- Greater flexibility: Implementations can be swapped without modifying dependent classes
- Clearer dependencies: Constructor signatures explicitly document required dependencies
- Easier refactoring: Component relationships are clearly defined
- Simpler dependency management: Easier control over resource initialization and lifecycle

Try not to over-abstract, but if there is already a Base Class (or interface) available, always try to use that (via dependency injection) in favor of a concrete implementation.

For example:

```ts
class SomeClass {
  // NOTE: We do not use concrete implementations (e.g. "BlockchainELectrum" or "StoreIndexedDB"), but the abstractions ("BaseBlockchain" and "BaseStore") instead.
  protected blockchain: BaseBlockchain;
  protected store: BaseStore;

  constructor(
    // Dependency Injection first
    dependencies: {
      blockchain: BaseBlockchain,
      store: BaseStore,
    },
    // Then main (required) initialization params
    initParams,
    // Then optional params (if applicable).
    opts,
  ) {
    this.blockchain = dependencies.blockchain;
    this.store = dependencies.store;
  }
}
```

### Factory Patterns

When creating classes, create static `from(...)` (or `fromSpecificThing(...)`) methods and place these at the top of the class (so that they appear first in the generated documentation).

In Javascript, a `constructor` cannot be asynchronous and we want to ensure that we can `await` class instantiation to mitigate against stateful errors.

For example:

```ts
class SomeClass {
  // Static "from" factory method.
  // NOTE: The below args are shorthand for using the same arguments as the constructor.
  //       Do not feel forced to use this - there are definitely some situations where you will have to define the args manually.
  //       But do try to follow the argument patterns in the above "Dependency Injection" section if possible.
  static async from(...args: ConstructorParameters<typeof SomeClass>) {
    // Create the instance.
    const someClass = new SomeClass(...args);

    // Execute the asynchronous initialization function.
    await someClass.start();

    // Return the instance.
    return someClass;
  }
}
```

Although this isn't always necessary (i.e. not all instantiations need to be `async`), it keeps instantiation of classes consistent.

*Semantically, think of "from" as meaning "instantiate from": This is more succinct than a `createFrom...` factory method which, if we have multiple ways of instantiation, can become verbose. E.g. `PrivateKey.fromWIF()` as opposed to `PrivateKey.createFromWIF()`. It also allows us to differentiate between actually "creating" something (e.g. a `Filesystem.createFile(...)`) versus "instantiating" a representation of a "thing".*

#### Extending from Static Factory Methods

If you anticipate that someone will `extend` a class and want to re-use your static factory method, this can be achieved as follows:

```ts
class YourClass {
  // NOTE: The generic T extends typeof YourClass (so that we can return the correct type).:
  static async from<T extends typeof YourClass>(
    // NOTE: "this" as first argument is what technically, allows the extended class to instantiate correctly.
    this: T,
    params: SomeParams = {},
  ) {
    // NOTE: So that Typescript can type correctly, we perform a cast.
    return new this(params) as InstanceType<T>;
  }

  // ...
}
```

Extended classes can then be instantiated seamlessly as follows:

```ts
class YourExtendedClass extends YourClass {
  // ...
}

// This will now be an instance of YourExtendedClass without having to redefine the `from(...)` method.
const yourExtClass = YourClass.from(...);
```

This adds quite a bit of confusing syntactic overhead, so only do this if you anticipate use-cases where extension is likely.

### EventEmitter Patterns for BaseAdapters

All BaseAdapters should extend EventEmitter. This design choice enables Stack components to work seamlessly with different frontend frameworks (vanilla JS, Vue, React, etc.) through a consistent event-based interface.

1. State Management
    Adapters in Stack should maintain minimal state. While backend flows typically operate procedurally (fetching data as needed), frontends often require reactive state updates for UI rendering.

2. Framework Agnostic
    Since reactive state handling varies across frameworks, we use event hooks rather than imposing specific state management patterns, allowing applications to handle higher-level state according to their framework (and product's) conventions.

To illustrate how this might be used with VueJS:

```ts
// Create a reactive variable (Vue example)
const isConnected = ref();

// Initialize adapter
const someAdapter = new SomeAdapter();

// Bind adapter state to reactive variable
someAdapter.on('isConnected', (isConnected) => {
  isConnected.value = isConnected;
});
```

```ts
// Declare a reactive variable for unspents (Vue example)
const unspents = ref([]);

// Initialize the wallet
const someWallet = await WalletHD.from(genesisData);

// Hook into state updated event
someWallet.on(
  'stateUpdated',
  async () => {
    // Sync unspents to the reactive variable
    unspents.value = await someWallet.updateUnspents(),
  },
  // (Optional) Debounce for 500ms to prevent excessive redraws
  500
);
```

**Note: While XO Wallet will provide convenience wrappers around these patterns, this represents the general approach to use in @xocash/stack.**

### Validations

Check types with TypeScript - Runtime type-checking is a code smell. If the function accepts a string, assume it's been given a string. TypeScript definitions should expose improperly called functions to the developer at compile time, don't re-implement it at runtime.

There are a few exceptions to this:

- If TypeScript's lack of dependant types makes validating certain properties of a type (e.g. the length of a Uint8Array), then runtime type-checking should be used to detect and return errors.
- If we are working with a network (remote/untrusted) payload, Type Validation should be done with [Zod](https://www.npmjs.com/package/zod).

#### Validating External Payloads

An example of working with Zod:

```ts
// Define a schema so that we can validate the payload.
export const SomeRemotePayload = z.object({
  publicKey: z.string(),
  action: z.enum(['set', 'delete', 'clear']),
  key: z.string(),
  value: z.string(),
  timestamp: z.number(),
});

// Run Zod's validation against this payload.
// NOTE: This will throw if it fails.
const parsedPayload = SomeRemotePayload.parse(rawPayload);

// (Optional) If this type is needed elsewhere, we can infer it from Zod and export it.
export type StoreRemoteHTTPEvent = z.infer<typeof StoreRemoteHTTPEventSchema>;
```

#### Leverage Primitives Over Repeated Validation

Avoid revalidation at every code-path: This is not only inefficient, but a giant code-smell. Instead, make use of Primitives as these provide assurances of correctness (Primitives are not just intended as a convenience - they are also intended to provide safety). For example, given a method:


```ts
// You can accept other types (this is a nice convenience).
broadcastTransaction(transaction: Transaction | Uint8Array | string): Transaction {
  // ... but always cast to the Primitive (to ensure correctness).
  if(!(transaction instanceof Transaction) {
    transaction = Transaction.from(transaction);
  }

  // ... use the Transaction primitive in follow up code.
}
```