From becae3775f5d245607f89e2fe2eae73150450837 Mon Sep 17 00:00:00 2001
From: Jos de Jong <wjosdejong@gmail.com>
Date: Wed, 13 Mar 2024 16:35:52 +0100
Subject: [PATCH] fix: #3175 cannot delete units using `math.Unit.deleteUnit`

---
 HISTORY.md                             |  4 ++--
 src/type/unit/Unit.js                  |  9 ++++++---
 test/unit-tests/type/unit/Unit.test.js | 23 +++++++++++++++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/HISTORY.md b/HISTORY.md
index 7dac6a8d2f..ce1d2954a8 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -8,10 +8,10 @@
 - Fix #3172: simplify `"true and true"`.
 - Fix #3163: `toTex` wrongly returning `Infinity` for large BigNumbers.
 - Fix #3162: add license information about CSParse (#3164).
-- Fix: expose `math.Unit.ALIASES` (see #3175).
+- Fix #3175: cannot delete units using `math.Unit.deleteUnit`.
 - Fix: faster startup time of the CLI and REPL by loading the bundle.
 - Fix: remove using polyfill.io inside the example
-  pretty_printing_with_mathjax.html (#3167). Thanks @SukkaW.
+  `pretty_printing_with_mathjax.html` (#3167). Thanks @SukkaW.
 
 
 # 2024-02-22, 12.4.0
diff --git a/src/type/unit/Unit.js b/src/type/unit/Unit.js
index 98b591d7b6..5c6fbd3e8a 100644
--- a/src/type/unit/Unit.js
+++ b/src/type/unit/Unit.js
@@ -2767,6 +2767,7 @@ export const createUnitClass = /* #__PURE__ */ factory(name, dependencies, ({
   }
 
   // aliases (formerly plurals)
+  // note that ALIASES is only used at creation to create more entries in UNITS by copying the aliased units
   const ALIASES = {
     meters: 'meter',
     inches: 'inch',
@@ -3327,8 +3328,8 @@ export const createUnitClass = /* #__PURE__ */ factory(name, dependencies, ({
       alias.name = aliasName
       Unit.UNITS[aliasName] = alias
     }
-    // delete the memoization cache, since adding a new unit to the array
-    // invalidates all old results
+
+    // delete the memoization cache because we created a new unit
     delete _findUnit.cache
 
     return new Unit(null, name)
@@ -3336,6 +3337,9 @@ export const createUnitClass = /* #__PURE__ */ factory(name, dependencies, ({
 
   Unit.deleteUnit = function (name) {
     delete Unit.UNITS[name]
+
+    // delete the memoization cache because we deleted a unit
+    delete _findUnit.cache
   }
 
   // expose arrays with prefixes, dimensions, units, systems
@@ -3344,7 +3348,6 @@ export const createUnitClass = /* #__PURE__ */ factory(name, dependencies, ({
   Unit.BASE_UNITS = BASE_UNITS
   Unit.UNIT_SYSTEMS = UNIT_SYSTEMS
   Unit.UNITS = UNITS
-  Unit.ALIASES = ALIASES
 
   return Unit
 }, { isClass: true })
diff --git a/test/unit-tests/type/unit/Unit.test.js b/test/unit-tests/type/unit/Unit.test.js
index 7b58b949e8..db6822e075 100644
--- a/test/unit-tests/type/unit/Unit.test.js
+++ b/test/unit-tests/type/unit/Unit.test.js
@@ -1324,6 +1324,29 @@ describe('Unit', function () {
     })
   })
 
+  describe('deleteUnit', function () {
+    it('should delete a unit', function () {
+      const math2 = math.create()
+
+      assert.strictEqual(math2.evaluate('5 b').toString(), '5 b')
+      assert.strictEqual(math2.evaluate('5 bytes').toString(), '5 bytes')
+      assert.strictEqual(math2.evaluate('5 byte').toString(), '5 byte') // alias of "bytes"
+
+      math2.Unit.deleteUnit('b')
+      math2.Unit.deleteUnit('bytes')
+      math2.Unit.deleteUnit('byte')
+
+      assert.throws(() => math2.evaluate('5 b').toString(), 'foo')
+      assert.throws(() => math2.evaluate('5 bytes').toString(), 'foo')
+      assert.throws(() => math2.evaluate('5 byte').toString(), 'foo')
+
+      // should not have changed the original math
+      assert.strictEqual(math.evaluate('5 b').toString(), '5 b')
+      assert.strictEqual(math.evaluate('5 bytes').toString(), '5 bytes')
+      assert.strictEqual(math.evaluate('5 byte').toString(), '5 byte')
+    })
+  })
+
   describe('splitUnit', function () {
     it('should split a unit into parts', function () {
       assert.strictEqual((new Unit(1, 'm')).splitUnit(['ft', 'in']).toString(), '3 ft,3.3700787401574765 in')