🏗 Firefly Shared Library Refactor - Phase No. 1

# 🏗 Firefly Shared Library Refactor - Phase No. 1 ###### tags: `iota` `firefly` `documentation` ###### last updated: 02/02/2022 ## Summary This document is a detailed plan of the first phase of the refactor of Firefly's shared library. It also includes some (loose) proposals for standardizing the way we organize code in aims of making it easier for developers to work in the codebase. This phase is mostly implementing support for changes that will be made in the second phase. The reason for doing it this way is to put all breaking changes in a single phase. ## Details ### Packages These high-level packages refer to groupings of the collection of composable source code _modules_ (detailed below), of which there are only __two__: `core` and `common`. :::info _Packages will live inside of the `packages/shared/lib/` directory._ ::: #### Core `core` refers to a group of modules that are the more infrastructural, integral parts the Firefly stack. These are things like communicating via the bridge with the actor system, platform handling with Electron and Capacitor, routing logic for the application, and so on. :::danger _These modules __must NOT__ depend on other modules outside of the `core` package!_ ::: Please see a full list [below](#Core-Modules). #### Common `common` refers to a group of modules that are intended to be consumed often and throughout the application (e.g. other library files and UI components). These are things like handling network clients, performing wallet operations, participating in events, interacting with Stronghold or Ledger, and so forth. __They are the domain-related business logic of the application.__ :::info _These modules __may__ depend on other modules both inside and outside of the `common` package._ ::: Please see a full list [below](#Common-Modules). ### Modules [Modules](https://www.typescriptlang.org/docs/handbook/modules.html) are composable collections of code components related to a specific concept, entity, functionality, and so on. Code components live in source code files scattered throughout a module. #### Structure & Components ##### Barrels A [barrel](https://basarat.gitbook.io/typescript/main-1/barrel) is an intermediary module that rolls up exports from other files and re-exports them. They are the `index.(ts|js)` files that live within modules. ##### Constants A constant is a value that __1)__ never changes and __2)__ is intended to be used throughout the application (i.e. Svelte components and other library files). ###### Conventions - All constants names __must__ be in `SCREAMING_SNAKE_CASE` - All constants __must__ be compile-time constants ###### Example ```typescript const MAX_NUM_IOTAS = 2_779_530_283_277_761 ``` ##### Enumerations An enumeration is an object that holds one or more variants of a certain type. ###### Conventions - All enum names __must__ be in `PascalCase` - All enum names __must__ be singular ###### Example ```typescript enum ProfileType { Stronghold = "stronghold", Ledger = "ledger", } ``` ##### Errors :::info _The following is still a work in progress._ ::: Our current `Promise`-based approach for handling errors is working fine, although it may be worth considering if it can be improved at all. We don't really have any standards as to what layer is best to throw errors on (API wrappers __only__, what about UI errors, etc.). Perhaps more global error types like `ComponentError`, `LibraryError`, and `ApiError` make sense as to knowing when, where, and how to throw what errors. We should have more domain-specific error groups for things like `StrongholdError`, `LedgerError`, or `NetworkError` that would potentially just `extend` the `LibraryError` type. Each type of error could have different implementation methods for handling, i.e. component errors and library errors are __always__ displayed to the user whereas an API error is _usually_ just logged to the console. Whatever is decided must be lightweight and generic for handling any and all sorts of data. We need to make use of specific API and library errors, e.g. StrongholdApiError and StrongholdLibraryError... all functions exposed fron Typescript later MUST have error handling if necessary. ###### Conventions _WIP_ ###### Examples _WIP_ ##### Functions A callable object that performs some type of operation. ###### Conventions - All functions __must__ be declared and defined with the `function` keyword; ES6 arrow-style functions __may__ be used in local instances like callbacks, lambdas, closures, and so on. - All functions __must__ be in `camelCase` - All static methods (in classes) __must__ be in `PascalCase` - All function signatures __must__ be explicitly typed (parameters _and_ return type); `unknown` __may__ be used when truly necessary while `any` __must NOT__ ever be used - All function name prefixes __should__ correspond with their return type and generally begin with verbs. - Functions that return a `boolean` __should__ be prefixed with a being verb (e.g. `isLedgerDeviceConnected()`, `has()`) - Functions that return `void` __should__ be prefixed with an action verb such that there is no expectation of data being returned (e.g. `backupStrongholdTo()`, `setClientOptions()`, `pollMarketData()`) - Functions that return none of the above __should__ be prefixed with an action verb such that there is an expectation of data being returned (e.g. `getNodeInfo()`, `createAccount()`, `generateMnemonic()`, `findMessage()`, `fetchMarketData()`) :::warning _It is entirely possible that the verb prefix rules __do NOT__ make sense in 100% of scenarios, in which case use your best judgement. The ultimate goal is just to create consistency :slightly_smiling_face:._ ::: ###### Example _TODO_ ##### Interfaces An interface is an object that declares a collection of functions that share some relation. ###### Conventions - All interface names __must__ be in `PascalCase` - All function signatures __must__ be explicitly typed ###### Example ```typescript interface StrongholdApi { lock(): Promise<void> setPassword(password: string): Promise<void> changePassword(oldPassword: string, newPassword: string): Promise<void> backupTo(path: string, password: string): Promise<void> restoreFrom(path: string, password: string): Promise<void> } ``` ##### Stores A store is an object that performs [CRUD](https://en.wikipedia.org/wiki/Create,_read,_update_and_delete) operations on data within a Svelte store. _WIP_ ###### Conventions - All store names __must__ be in `camelCase` :::warning _There is __no__ distinction between regular variables and store variables. It would be worth exploring naming conventions to accomplish this._ ::: Possible ideas are: - Prefix store variable names with an underscore, e.g. `_isStrongholdLocked` - Use `PascalCase` for store variables, e.g. `IsStrongholdLocked` (usually this is convention for `static` variables, but we don't use those much anyway :thinking_face:) ###### Example ```typescript const isStrongholdLocked = writable<boolean>(true) // usage in TS file if (get(isStrongholdLocked)) { // do something... isStrongholdLocked.set(true) } // usage in Svelte component if ($isStrongholdLocked) { // do something... isStrongholdLocked.set(true) } ``` ##### Tests A test is a file containing multiple unit tests for the functions in its corresponding source code file. The tests within a module form a test suite. ###### Conventions Please refer to the [testing wiki](https://github.com/iotaledger/firefly/wiki/Guide:-Testing) to see coding conventions of unit tests. ###### Example Please refer to [this file](https://github.com/iotaledger/firefly/blob/develop/packages/shared/lib/tests/currency.test.ts) to see an example of a file with unit tests. ##### Types A type is a definition of the data contained within a particular object. ###### Conventions - All type names __must__ be in `PascalCase` - All type definitions __must__ use the `type` keyword (__do NOT__ use `interface`) - Any known, non-primitive type __must__ be explicitly defined; `unknown` __should__ be used sparingly while `any` __must NOT__ be used at all ###### Example ```typescript type NodeAuth { username?: string password?: string jwt?: string } type Node { url: string auth?: NodeAuth isPrimary: boolean isDisabled: boolean } ``` #### Module API Design _WIP_ ##### Comments We have already put some effort into writing nice TSDoc comments for some but not all of our functions, and it shouldn't go to waste. Idea: I think we can benefit from such comments as developers typically first resort to documentation or a specification rather than the code itself. We could theoretically generate specs for the library and present them in the GH wiki for devs to comb through when they need it. It would be much easier to write a shortened, always up-to-date list of available functions and where they live rather than having to search through the code manually. TODO: Write more finalized section for comments + add examples. Bad comment example (too redundant and obvious): ```typescript /** * Type definition for application settings */ export interface AppSettings { deepLinking: boolean language: string theme: AppTheme darkMode: boolean notifications: boolean sendDiagnostics: boolean } ``` Better comment (concisely gives context and is more informative): ```typescript /** * General settings for the application, used * across multiple profiles. */ export interface AppSettings { deepLinking: boolean language: string theme: AppTheme darkMode: boolean notifications: boolean sendDiagnostics: boolean } ``` A comment should explain hard-to-follow logic or framework specific code. ```svelte $: profileName, (error = '') // Error clears when profileName changes ``` A comment can refer to additional material that helps explain difficult parts of the code. ```typescript // Fisher-Yates Shuffle (https://waimin.me/Generate_unique_randoms/) private generateIndices(): void { const randomIndices: number[] = []; const numbers = [...Array(24).keys()]; for (let i = 0; i < 3; i += 1) { const randomIndex = i + Math.floor(Math.random() * (24 - i)); const randomNumber = numbers[randomIndex]; randomIndices.push(randomNumber); numbers[randomIndex] = numbers[i]; numbers[i] = randomNumber; } randomIndices = randomIndices.sort((a, b) => a - b); } ``` A comment can also be used in sections where more development has to happen in the future. These area's must be marked with TODO, so that it is easy to query. ```typescript // TODO: ledger, The operand of a 'delete' operator cannot be a read-only property $: if (!$isSoftwareProfile) { delete securitySettings.ExportStronghold delete securitySettings.ChangePassword } ``` #### Wallet API Usage ##### UI Services Services are mechanisms that allow UI components to mutate the state of an application. Their purpose is provide a layer of abstraction between the components and the underlying library as to keep a distinct separation between business logic and interface logic. By using an abstraction, we can remove all side-effects away from the API calls and keep them pure. This is likely to break the application in many places, but if we persist through the mud it'll be well worth it. This is more likely it's own refactor, but something to note. Services have the following requirements: - They __must__ provide access or expose store variables as a means to maintain the magic of Svelte reactivity - ... :::warning The concept of "Service" is subject to change; the general idea is to create a layer of abstraction between the UI components and the TypeScript library. ::: ##### Wrapper Functions :::danger _It is ideal to avoid accessing or using the bound `api` object directly. Use a wrapper function instead!_ ::: We should aim to write wrapper functions around each endpoint that adhere to the following guidelines: - All functions __must__ explicitly return a type (even if `void`) - All functions __must__ support optional `onSuccess` and `onError` callbacks* - All functions __must__ be free of [side-effects](https://en.wikipedia.org/wiki/Side_effect_(computer_science))** - All `Promise`-based functions __must__ be prefixed with `async` \* We should think about typing at least the callbacks of the function, perhaps the arguments to pass to the wrapper function as well. \** It could be likely that this constraint introduces breaking changes as some of the functions that contain side-effects are used in more than one place. It is in these circumstances that passing the optional success and error callback functions is preferable. We need to figure out a way of passing or keeping callbacks that some of these functions already have (that have side effects). ##### Messages, Responses, and Events There are a few key components the application uses to communicate with the wallet and extension actors within the backend actor system. They are... 1. `Message` - a request sent to the actor system 2. `Response` - the corresponding response to a `Message`, sent back to the requester 3. `Event` - an emitted even when particular things happen (e.g. stronghold status changes, receives new transaction, balance changes) We should also create separation between message responses and events. Currently we use `Event<T>` to describe all API endpoint return types, although these should just be used for the actual events emitted from the wallet actor. As the application listens for general events but waits for and expects specific responses, it would be more appropriate to use `Event<T>` and `Response<T>` respectively. As such we should also separate the enumerations of both of these types, i.e. `EventType` and `ResponseType`. Currently both event and message responses are inside of `ResponseType` (which is used inside of an `Event`). ### Module Lists The following are comprehensive lists of all modules in the high-level packages. #### Core Modules - `api` - defines global API object methods - `app` - manages application settings, updating, and operations (e.g. login, logout) - `bridge` - communicates to backend with `Actor` via `BridgeMessages` - `i18n` - handles internationalization and locale logic - `notification` - signals user of results of their actions - `platform` - houses lower-level platform operations and setup - `popup` - base infrastructure for displaying popup UI - `router` - handles application routing logic - `shell` - manages API callbacks and executes them when response is received - `time` - houses constants and functions useful for dealing with time - `utils` - houses utility functions (i.e. random, sleep, pick, types, etc.) - `validation` - validates messages and responses to and from the backend #### Common Modules - `iota-links` - handles deep link requests from web pages or extensions - `ledger` - manages connection and operations with Ledger device - `market` - interacts and handles market data (e.g. prices, conversions, timelines) - `migration` - moves funds from legacy mainnet to Chrysalis mainnet - `network` - manages network configuration and client options - `participation` - interacts with voting and staking events - `profile` - handles user profile operations - `storage` - creates and modifies storage objects - `stronghold` - manages security operations with Stronghold - `wallet` - manages accounts and handling value transfers (+ receiving) :::warning _These lists are still a WIP._ ::: ### Path Aliases These will help us significantly with the cleanliness and readability of our import statements. It should also make it easier to understand where a given function, file, etc. is. I propose we use the following: - `@common` - `packages/shared/lib/common/` - `@components` - `packages/shared/components/` - `@core` - `packages/shared/lib/core/` :::warning _To keep disruptive changes contained, this phase will __ONLY__ add support for path aliases; __NOT__ changes that use them_. ::: ### Testing #### Coverage While 100% test coverage would be ideal, it should be acceptable to reach 80+%. There may be circumstances where it is exceedingly difficult and just not worth the time to reach 80%... that's :ok_hand:. :::danger _Any module or functionality handling value transfer __should__ be tested. We've been fortunate in having minimal problems with our current approach, but should not rely on luck to keep our heads above water._ ::: #### Mocking There are a few key components that need mocking to successfully test some of Firefly's key functionality. These are... - `wallet.rs` bindings / global `api` object - I18N locale support, which is used in some functions that format compute and format strings (focusing on `localize` function) - More browser- or DOM-related objects or properties (e.g. `matchMedia`) #### Unit Testing The significant change for unit tests is that they, like the flat directory of source code files, will be broken up and placed into their corresponding modules. Please refer to the [testing guide](https://github.com/iotaledger/firefly/wiki/Guide:-Testing) from the Firefly developer wiki to see how they are written. #### Integration Testing (opt.) Implementing support for integration tests would be quite an engineering task (one that is out-of-scope for this refactor) but it worth mentioning as doing so would be massively beneficial for keeping Firefly's production release as stable as possible. Put simply, we _could_ (read: __should__) be writing test suites or scripts that test important things like API endpoints, storage or profile operations, and so on. This could be made possible by the combination of GitHub CI workflows and scripts that would setup the infrastructure of Firefly similar to how it is built for releases. ### Error Handling _WIP_ ## Team Questions - What is the best way to create an abstraction layer between the UI and the TS library? In React, there are actions and reducers that operate on state and are intended to be used by the UI. In Angular, there are singleton services (not just for modifying or reading state) that are injected into UI components (e.g. there would be a `StrongholdService` that exposes functions with side-effects using the underlying API wrapper functions that have NO side-effects). The idea is that API functions or state is not directly accessible to the UI, keeping components purely presentational with minimal logic (other than logic for UI). The problem with this is with maintaining reactivity within Svelte components. - What is a good naming convention for Svelte store variables that makes them immediately distinct from regular variables? - Should we use `kebab-case` or `camelCase` for naming modules and files? - Is there any preference of import sorting order? E.g. simply alphabetical, first Svelte > third-part > local lib and alphabetically sorting each of those groups, etc. - Should we writing functions as declarations (ES5, `function` keyword) or as expressions (ES6, `=>` syntax)? Read [here](https://medium.com/@zac_heisey/es5-vs-es6-functions-cb51536002ee) for more info. - __David__: We should also think about standardizing our use of functions in general. Sometimes we use arrow functions, sometimes regular functions. My preference goes to regular functions, since this allows us to place all variables on the top of a file and the functions under them. However, I understand that for some use cases (one-line functions), it might make sense to use them. We can discuss this and I can write the final conclusion here. ## Notes - __What should we do about the abstraction layer between the UI components and the library?__ There is an excellent article [here](https://medium.com/geekculture/lets-talk-about-state-management-in-svelte-e4a59057b990) with more detail (another one [here](https://vsavkin.com/managing-state-in-angular-2-applications-caf78d123d02)). ## Other Parts - [Overview](https://hackmd.io/nzaGPEY5QnuaOomOoz5MCg) - [WIP] [Phase No. 2](https://hackmd.io/4VBXE7u7RyeD9kgjPV3tgQ)

Syntax	Example	Reference
# Header	Header	基本排版
- Unordered List	Unordered List
1. Ordered List	Ordered List
- [ ] Todo List	Todo List
> Blockquote	Blockquote
Bold font	Bold font
Italics font	Italics font
~~Strikethrough~~	~~Strikethrough~~
19^th^	19^th
H~2~O	H₂O
++Inserted text++	Inserted text
==Marked text==	Marked text
[link text](https:// "title")	Link
![image alt](https:// "title")	Image
`Code`	`Code`	在筆記中貼入程式碼
```javascript var i = 0; ```	`var i = 0;`
:smile:		Emoji list
{%youtube youtube_id %}	Externals
$L^aT_eX$	L^aT_eX
:::info This is a alert area. :::	This is a alert area.