Skip to content

Commit

Permalink
fix: #2138 linter issues with fail function (#2171)
Browse files Browse the repository at this point in the history
* fix: #2138 linter issues with fail function

- refactoring fail function to be a custom Error class
- adjusting throw class given the above changes
- adding missing imports
- removing unused imports

* adding tests

* typo
  • Loading branch information
JuanJo4 authored Apr 1, 2024
1 parent 9577fc7 commit f87f96d
Show file tree
Hide file tree
Showing 28 changed files with 155 additions and 121 deletions.
3 changes: 2 additions & 1 deletion __tests__/core/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
} from "../../src"

import { autorun, reaction, observable, configure, getDebugName } from "mobx"
import { MstError } from "../../src/internal"

const createTestFactories = () => {
const Factory = types
Expand Down Expand Up @@ -887,7 +888,7 @@ test("#993-1 - after attach should have a parent when accesing a reference direc
})
.actions((self) => ({
afterAttach() {
throw fail("should never be called")
throw new MstError("should never be called")
}
}))

Expand Down
20 changes: 20 additions & 0 deletions __tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { MstError } from "../src/utils"

describe("MstError custom error class", () => {
test("with default message", () => {
const error = new MstError()
expect(error.message).toBe("[mobx-state-tree] Illegal state")
})

test("with custom message", () => {
const customMessage = "custom error message"
const error = new MstError(customMessage)
expect(error.message).toBe(`[mobx-state-tree] ${customMessage}`)
})

test("instance of MstError", () => {
const error = new MstError()
expect(error).toBeInstanceOf(MstError)
expect(error).toBeInstanceOf(Error)
})
})
6 changes: 3 additions & 3 deletions src/core/action.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { action as mobxAction } from "mobx"
import {
getStateTreeNode,
fail,
MstError,
argsToArray,
IDisposer,
getRoot,
Expand Down Expand Up @@ -292,12 +292,12 @@ function runMiddleWares(
if (devMode()) {
if (!nextInvoked && !abortInvoked) {
const node2 = getStateTreeNode(call.tree)
throw fail(
throw new MstError(
`Neither the next() nor the abort() callback within the middleware ${handler.name} for the action: "${call.name}" on the node: ${node2.type.name} was invoked.`
)
} else if (nextInvoked && abortInvoked) {
const node2 = getStateTreeNode(call.tree)
throw fail(
throw new MstError(
`The next() and abort() callback within the middleware ${handler.name} for the action: "${call.name}" on the node: ${node2.type.name} were invoked.`
)
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/flow.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { argsToArray, fail, setImmediateWithFallback } from "../utils"
import { argsToArray, MstError, setImmediateWithFallback } from "../utils"
import {
FunctionWithFlag,
getCurrentActionContext,
Expand Down Expand Up @@ -98,11 +98,11 @@ export function createFlowSpawner(name: string, generator: FunctionWithFlag) {
const runId = getNextActionId()
const parentContext = getCurrentActionContext()!
if (!parentContext) {
throw fail("a mst flow must always have a parent context")
throw new MstError("a mst flow must always have a parent context")
}
const parentActionContext = getParentActionContext(parentContext)
if (!parentActionContext) {
throw fail("a mst flow must always have a parent action context")
throw new MstError("a mst flow must always have a parent action context")
}

const contextBase = {
Expand Down Expand Up @@ -193,7 +193,7 @@ export function createFlowSpawner(name: string, generator: FunctionWithFlag) {
// TODO: support more type of values? See https://github.com/tj/co/blob/249bbdc72da24ae44076afd716349d2089b31c4c/index.js#L100
if (!ret.value || typeof ret.value.then !== "function") {
// istanbul ignore next
throw fail("Only promises can be yielded to `async`, got: " + ret)
throw new MstError("Only promises can be yielded to `async`, got: " + ret)
}
return ret.value.then(onFulfilled, onRejected)
}
Expand Down
7 changes: 4 additions & 3 deletions src/core/json-patch.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fail, stringStartsWith } from "../internal"
import { MstError, stringStartsWith } from "../internal"

/**
* https://tools.ietf.org/html/rfc6902
Expand All @@ -19,7 +19,8 @@ export interface IReversibleJsonPatch extends IJsonPatch {
* @hidden
*/
export function splitPatch(patch: IReversibleJsonPatch): [IJsonPatch, IJsonPatch] {
if (!("oldValue" in patch)) throw fail(`Patches without \`oldValue\` field cannot be inversed`)
if (!("oldValue" in patch))
throw new MstError(`Patches without \`oldValue\` field cannot be inversed`)
return [stripPatch(patch), invertPatch(patch)]
}

Expand Down Expand Up @@ -127,7 +128,7 @@ export function splitJsonPath(path: string): string[] {
stringStartsWith(path, "./") ||
stringStartsWith(path, "../")
if (!valid) {
throw fail(`a json path must be either rooted, empty or relative, but got '${path}'`)
throw new MstError(`a json path must be either rooted, empty or relative, but got '${path}'`)
}

// '/a/b/c' -> ["a", "b", "c"]
Expand Down
16 changes: 8 additions & 8 deletions src/core/mst-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
splitJsonPath,
asArray,
EMPTY_OBJECT,
fail,
MstError,
IDisposer,
resolveNodeByPath,
getRelativePathBetweenNodes,
Expand Down Expand Up @@ -268,7 +268,7 @@ export function protect(target: IAnyStateTreeNode): void {
assertIsStateTreeNode(target, 1)

const node = getStateTreeNode(target)
if (!node.isRoot) throw fail("`protect` can only be invoked on root nodes")
if (!node.isRoot) throw new MstError("`protect` can only be invoked on root nodes")
node.isProtectionEnabled = true
}

Expand Down Expand Up @@ -301,7 +301,7 @@ export function unprotect(target: IAnyStateTreeNode): void {
assertIsStateTreeNode(target, 1)

const node = getStateTreeNode(target)
if (!node.isRoot) throw fail("`unprotect` can only be invoked on root nodes")
if (!node.isRoot) throw new MstError("`unprotect` can only be invoked on root nodes")
node.isProtectionEnabled = false
}

Expand Down Expand Up @@ -394,7 +394,7 @@ export function getParent<IT extends IAnyStateTreeNode | IAnyComplexType>(
if (--d === 0) return parent.storedValue as any
parent = parent.parent
}
throw fail(`Failed to find the parent of ${getStateTreeNode(target)} at depth ${depth}`)
throw new MstError(`Failed to find the parent of ${getStateTreeNode(target)} at depth ${depth}`)
}

/**
Expand Down Expand Up @@ -437,7 +437,7 @@ export function getParentOfType<IT extends IAnyComplexType>(
if (type.is(parent.storedValue)) return parent.storedValue
parent = parent.parent
}
throw fail(`Failed to find the parent of ${getStateTreeNode(target)} of a given type`)
throw new MstError(`Failed to find the parent of ${getStateTreeNode(target)} of a given type`)
}

/**
Expand Down Expand Up @@ -577,7 +577,7 @@ export function tryReference<N extends IAnyStateTreeNode>(
return isAlive(node) ? node : undefined
}
} else {
throw fail("The reference to be checked is not one of node, null or undefined")
throw new MstError("The reference to be checked is not one of node, null or undefined")
}
} catch (e) {
if (e instanceof InvalidReferenceError) {
Expand Down Expand Up @@ -605,7 +605,7 @@ export function isValidReference<N extends IAnyStateTreeNode>(
} else if (isStateTreeNode(node)) {
return checkIfAlive ? isAlive(node) : true
} else {
throw fail("The reference to be checked is not one of node, null or undefined")
throw new MstError("The reference to be checked is not one of node, null or undefined")
}
} catch (e) {
if (e instanceof InvalidReferenceError) {
Expand Down Expand Up @@ -777,7 +777,7 @@ export function getEnv<T = any>(target: IAnyStateTreeNode): T {

const node = getStateTreeNode(target)
const env = node.root.environment
if (!env) throw fail(`Failed to find the environment of ${node} ${node.path}`)
if (!env) throw new MstError(`Failed to find the environment of ${node} ${node.path}`)
return env
}

Expand Down
6 changes: 4 additions & 2 deletions src/core/node/BaseNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
IAnyType,
IDisposer,
devMode,
fail
MstError
} from "../../internal"
import { createAtom, IAtom } from "mobx"

Expand Down Expand Up @@ -164,7 +164,9 @@ export abstract class BaseNode<C, S, T> {
if (devMode()) {
if (!this.isAlive) {
// istanbul ignore next
throw fail("assertion failed: cannot finalize the creation of a node that is already dead")
throw new MstError(
"assertion failed: cannot finalize the creation of a node that is already dead"
)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/core/node/create-node.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
fail,
MstError,
ObjectNode,
ScalarNode,
AnyNode,
Expand All @@ -24,7 +24,7 @@ export function createObjectNode<C, S, T>(
if (existingNode) {
if (existingNode.parent) {
// istanbul ignore next
throw fail(
throw new MstError(
`Cannot add an object to a state tree if it is already part of the same or another state tree. Tried to assign an object to '${
parent ? parent.path : ""
}/${subpath}', but it lives already at '${existingNode.path}'`
Expand Down
6 changes: 3 additions & 3 deletions src/core/node/identifier-cache.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { IObservableArray, values, observable, entries } from "mobx"
import { fail, ObjectNode, mobxShallow, AnyObjectNode, IAnyComplexType } from "../../internal"
import { MstError, ObjectNode, mobxShallow, AnyObjectNode, IAnyComplexType } from "../../internal"

let identifierCacheId = 0

Expand Down Expand Up @@ -37,7 +37,7 @@ export class IdentifierCache {
this.cache.set(identifier, observable.array<AnyObjectNode>([], mobxShallow))
}
const set = this.cache.get(identifier)!
if (set.indexOf(node) !== -1) throw fail(`Already registered`)
if (set.indexOf(node) !== -1) throw new MstError(`Already registered`)
set.push(node)
if (lastCacheUpdate) {
this.updateLastCacheModificationPerId(identifier)
Expand Down Expand Up @@ -114,7 +114,7 @@ export class IdentifierCache {
case 1:
return matches[0]
default:
throw fail(
throw new MstError(
`Cannot resolve a reference to type '${
type.name
}' with id: '${identifier}' unambigously, there are multiple candidates: ${matches
Expand Down
8 changes: 4 additions & 4 deletions src/core/node/node-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {
fail,
MstError,
ObjectNode,
splitJsonPath,
joinJsonPath,
Expand Down Expand Up @@ -89,7 +89,7 @@ export function assertIsStateTreeNode(
export function getStateTreeNode(value: IAnyStateTreeNode): AnyObjectNode {
if (!isStateTreeNode(value)) {
// istanbul ignore next
throw fail(`Value ${value} is no MST Node`)
throw new MstError(`Value ${value} is no MST Node`)
}
return value.$treenode!
}
Expand Down Expand Up @@ -119,7 +119,7 @@ const doubleDot = (_: any) => ".."
export function getRelativePathBetweenNodes(base: AnyObjectNode, target: AnyObjectNode): string {
// PRE condition target is (a child of) base!
if (base.root !== target.root) {
throw fail(
throw new MstError(
`Cannot calculate relative path: objects '${base}' and '${target}' are not part of the same object tree`
)
}
Expand Down Expand Up @@ -182,7 +182,7 @@ export function resolveNodeByPathParts(
}
}
}
throw fail(
throw new MstError(
`Could not resolve '${part}' in path '${
joinJsonPath(pathParts.slice(0, i)) || "/"
}' while resolving '${joinJsonPath(pathParts)}'`
Expand Down
26 changes: 13 additions & 13 deletions src/core/node/object-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
createActionInvoker,
EMPTY_OBJECT,
extend,
fail,
MstError,
freeze,
IAnyType,
IdentifierCache,
Expand Down Expand Up @@ -155,7 +155,7 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
}

if (typeof id !== "string" && typeof id !== "number") {
throw fail(
throw new MstError(
`Instance identifier '${this.identifierAttribute}' for type '${this.type.name}' must be a string or a number`
)
}
Expand All @@ -182,7 +182,7 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
if (devMode()) {
if (this.state !== NodeLifeCycle.INITIALIZING) {
// istanbul ignore next
throw fail(
throw new MstError(
"assertion failed: the creation of the observable instance must be done on the initializing phase"
)
}
Expand Down Expand Up @@ -294,25 +294,25 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
if (devMode()) {
if (!subpath) {
// istanbul ignore next
throw fail("assertion failed: subpath expected")
throw new MstError("assertion failed: subpath expected")
}
if (!newParent) {
// istanbul ignore next
throw fail("assertion failed: new parent expected")
throw new MstError("assertion failed: new parent expected")
}

if (this.parent && parentChanged) {
throw fail(
throw new MstError(
`A node cannot exists twice in the state tree. Failed to add ${this} to path '${newParent.path}/${subpath}'.`
)
}
if (!this.parent && newParent.root === this) {
throw fail(
throw new MstError(
`A state tree is not allowed to contain itself. Cannot assign ${this} to path '${newParent.path}/${subpath}'`
)
}
if (!this.parent && !!this.environment && this.environment !== newParent.root.environment) {
throw fail(
throw new MstError(
`A state tree cannot be made part of another state tree as long as their environments are different.`
)
}
Expand Down Expand Up @@ -394,7 +394,7 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
const error = this._getAssertAliveError(context)
switch (livelinessChecking) {
case "error":
throw fail(error)
throw new MstError(error)
case "warn":
warnError(error)
}
Expand Down Expand Up @@ -460,7 +460,7 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
assertWritable(context: AssertAliveContext): void {
this.assertAlive(context)
if (!this.isRunningAction() && this.isProtected) {
throw fail(
throw new MstError(
`Cannot modify '${this}', the object is protected and can only be modified by using an action.`
)
}
Expand Down Expand Up @@ -497,7 +497,7 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
}

detach(): void {
if (!this.isAlive) throw fail(`Error while detaching, node is not alive.`)
if (!this.isAlive) throw new MstError(`Error while detaching, node is not alive.`)

this.clearParent()
}
Expand Down Expand Up @@ -607,12 +607,12 @@ export class ObjectNode<C, S, T> extends BaseNode<C, S, T> {
this._internalEventsRegister(InternalEvents.Dispose, disposer, true)
return
}
throw fail("cannot add a disposer when it is already registered for execution")
throw new MstError("cannot add a disposer when it is already registered for execution")
}

removeDisposer(disposer: () => void): void {
if (!this._internalEventsHas(InternalEvents.Dispose, disposer)) {
throw fail("cannot remove a disposer which was never registered for execution")
throw new MstError("cannot remove a disposer which was never registered for execution")
}
this._internalEventsUnregister(InternalEvents.Dispose, disposer)
}
Expand Down
Loading

0 comments on commit f87f96d

Please sign in to comment.