diff --git a/src/diagram/custom-math-js.test.ts b/src/diagram/custom-math-js.test.ts index ae5d738..58828d1 100644 --- a/src/diagram/custom-math-js.test.ts +++ b/src/diagram/custom-math-js.test.ts @@ -1,9 +1,18 @@ import { createMath } from "./custom-mathjs"; import { UnitsManager } from "./units-manager"; +import { IMathLib } from "./utils/mathjs-utils"; describe("MathJS", () => { - const unitsManager = new UnitsManager(); - const { evaluate, unit } = createMath(unitsManager); + let unitsManager: UnitsManager; + let evaluate: IMathLib["evaluate"]; + let unit: IMathLib["unit"]; + + beforeEach(() => { + unitsManager = new UnitsManager(); + const math = createMath(unitsManager); + evaluate = math.evaluate; + unit = math.unit; + }); it("can handle unit conversion with evaluate and unit values", () => { const scope = { @@ -213,6 +222,40 @@ describe("MathJS", () => { expect(simpl.toString()).toEqual("1 m"); }); + it("handles '_'", () => { + unitsManager.addUnit("t_s"); + const localMath = createMath(unitsManager); + const scope = { + a: localMath.unit(1, "m/t_s"), + b: localMath.unit(1, "t_s") + }; + const result = localMath.evaluate("a*b", scope); + const simpl = result.simplify(); + expect(simpl.toString()).toEqual("1 m"); + }); + + // This section describes the valid variable characters: + // https://mathjs.org/docs/expressions/syntax.html#constants-and-variables + it("handles a valid variable which is an invalid unit", () => { + unitsManager.addUnit("Ā"); + const localMath = createMath(unitsManager); + const scope = { + a: localMath.unit(1, "m") + }; + const result = localMath.evaluate("a", scope); + expect(result.toString()).toEqual("1 m"); + }); + + it("handles an invalid variable", () => { + unitsManager.addUnit("✔"); + const localMath = createMath(unitsManager); + const scope = { + a: localMath.unit(1, "m") + }; + const result = localMath.evaluate("a", scope); + expect(result.toString()).toEqual("1 m"); + }); + it("handles units with options", () => { unitsManager.addUnit("cat", { aliases: ["cats"]}); const localMath = createMath(unitsManager); diff --git a/src/diagram/models/variable.test.ts b/src/diagram/models/variable.test.ts index b7b343c..aba2a6c 100644 --- a/src/diagram/models/variable.test.ts +++ b/src/diagram/models/variable.test.ts @@ -338,27 +338,12 @@ describe("Variable", () => { {id: "variable", inputs: ["input"], expression: "a to cm/things"} ] }); - container.unitsManager.addUnit("thing", { aliases: ["things"] }); const variable = container.items[1] as VariableType; expect(variable.computedValueIncludingMessageAndError).toEqual({value: 900}); expect(variable.computedUnitIncludingMessageAndError).toEqual({unit: "cm / things"}); }); - it("with a invalid unit in A it returns without crashing", () => { - const container = VariablesContainer.create({ - items: [ - {id: "input", name: "a", value: 9, unit: "m/"}, - {id: "variable", inputs: ["input"], expression: "a to cm"} - ] - }); - const variable = container.items[1] as VariableType; - - expect(variable.computedValueIncludingMessageAndError.message?.short).toEqual("Warning: cannot compute value from inputs"); - expect(variable.computedUnitIncludingMessageAndError.error?.short).toEqual("Warning: invalid input units"); - - }); - it("with 2 inputs with units and operation multiply it returns result", () => { const container = VariablesContainer.create({ items: [ @@ -644,20 +629,71 @@ describe("Variable", () => { expect(variable.computedValueIncludingMessageAndError).toEqual({value: 0.01}); }); - it("handles invalid units", () => { - // This can happen when a user is typing a unit - const container = VariablesContainer.create({ - items: [ - {id: "inputA", name: "a", value: 1, unit: "m/"}, - {id: "inputB", name: "b", value: 100, unit: "s"}, - {id: "variable", inputs: ["inputA", "inputB"], - expression: "a*b"} - ] - }); - const variable = container.items[2] as VariableType; - - expect(variable.computedUnitIncludingMessageAndError.error?.short).toEqual("Warning: invalid input units"); - expect(variable.computedValueIncludingMessageAndError.message?.short).toEqual("Warning: cannot compute value from inputs"); + describe("invalid units", () => { + it("handles incomplete unit strings", () => { + // This can happen when a user is typing a unit + const container = VariablesContainer.create({ + items: [ + {id: "inputA", name: "a", value: 1, unit: "m/"}, + {id: "inputB", name: "b", value: 100, unit: "s"}, + {id: "variable", inputs: ["inputA", "inputB"], + expression: "a*b"} + ] + }); + const variable = container.items[2] as VariableType; + + expect(variable.computedUnitIncludingMessageAndError.error?.short).toEqual("Warning: invalid input units"); + expect(variable.computedValueIncludingMessageAndError.message?.short).toEqual("Warning: cannot compute value from inputs"); + }); + + it("handles incomplete unit conversion without crashing", () => { + const container = VariablesContainer.create({ + items: [ + {id: "input", name: "a", value: 9, unit: "m/"}, + {id: "variable", inputs: ["input"], expression: "a to cm"} + ] + }); + const variable = container.items[1] as VariableType; + + expect(variable.computedValueIncludingMessageAndError.message?.short).toEqual("Warning: cannot compute value from inputs"); + expect(variable.computedUnitIncludingMessageAndError.error?.short).toEqual("Warning: invalid input units"); + + }); + + // Currently we only allow units to be letters, `$`, and `_` + // Numbers are allowed if they aren't the first character + // A string which is not a valid "variable" will go through one code path. + // A string which is a valid "variable" will go through a different code path. + // The char `Ā` is a valid variable, but not a valid unit. + // This section describes the valid variable characters: + // https://mathjs.org/docs/expressions/syntax.html#constants-and-variables + it("handles valid variable which is not a valid unit", () => { + const container = VariablesContainer.create({ + items: [ + {id: "inputA", name: "a", value: 1, unit: "Ā"}, + {id: "variable", inputs: ["inputA"], + expression: "a"} + ] + }); + const variable = container.items[1] as VariableType; + + expect(variable.computedUnitIncludingMessageAndError.error?.short).toEqual("Warning: invalid input units"); + expect(variable.computedValueIncludingMessageAndError.message?.short).toEqual("Warning: cannot compute value from inputs"); + }); + + it("handles invalid variable character", () => { + const container = VariablesContainer.create({ + items: [ + {id: "inputA", name: "a", value: 1, unit: "✔"}, + {id: "variable", inputs: ["inputA"], + expression: "a"} + ] + }); + const variable = container.items[1] as VariableType; + + expect(variable.computedUnitIncludingMessageAndError.error?.short).toEqual("Warning: invalid input units"); + expect(variable.computedValueIncludingMessageAndError.message?.short).toEqual("Warning: cannot compute value from inputs"); + }); }); it("handles adding compatible inputs first with a value and second without a value", () => { diff --git a/src/diagram/units-manager.ts b/src/diagram/units-manager.ts index 3b12dd9..7c708e6 100644 --- a/src/diagram/units-manager.ts +++ b/src/diagram/units-manager.ts @@ -32,7 +32,7 @@ export class UnitsManager { const isAlphaOriginal = mathUnit.isValidAlpha; mathUnit.isValidAlpha = function (c: string): boolean { - return isAlphaOriginal(c) || c === "$"; + return isAlphaOriginal(c) || c === "$" || c === "_"; }; deleteUnits.forEach((u: string) => mathUnit.deleteUnit(u)); @@ -42,10 +42,16 @@ export class UnitsManager { // createUnit(units: Record, options?: CreateUnitOptions): Unit; // If u.options is undefined and we call `m.createUnit(u.unit, u.options);` Typescript's overloading // support fails for some reason. So this is split out to 2 function calls. - if (u.options) { - m.createUnit(u.unit, u.options); - } else { - m.createUnit(u.unit); + try { + if (u.options) { + m.createUnit(u.unit, u.options); + } else { + m.createUnit(u.unit); + } + } catch (e) { + // Ignore any invalid units. If we remove them from the units array that will trigger loop. + // In practice no invalid units should be added because they should be skipped during the call + // to `getMathUnit`. } }); @@ -91,6 +97,9 @@ export class UnitsManager { try { const isValidUnknownUnitName = (name: string) => { // MathJS doesn't handle empty string units. + // NOTE: this isn't really checking if the string is valid, it is just making sure it isn't + // empty and is unknown. The Unit library is more strict about what is a valid unit, + // however there isn't an easy way to access this without just trying to create a Value return name && !mathLib.Unit.isValuelessUnit(name); }; @@ -99,7 +108,6 @@ export class UnitsManager { }; const registerUnit = (name: string, options?: CreateUnitOptions) => { - this.addUnit(name, options); // NOTE: this `createUnit` call only adds the unit to the passed in mathLib. // Each variable has its own mathLib, so the other variable's mathLibs are not // updated here. @@ -111,6 +119,11 @@ export class UnitsManager { // mathLib.createUnit fails if options is undefined mathLib.createUnit(name); } + + // This is called after the createUnit call on purpose. The createUnit call above will + // throw an exception if the unit is invalid. In that case this addUnit call will never + // happen. So the invalid unit will not be added to the unit manager. + this.addUnit(name, options); }; // Look for unknown units in the unit string