woodser / monero-ts

TypeScript library for using Monero
http://woodser.github.io/monero-ts/typedocs
MIT License
207 stars 70 forks source link

New `getFs` code enforces memfs in browsers #241

Open mainnet-pat opened 2 weeks ago

mainnet-pat commented 2 weeks ago

With the new code I can not polyfill fs with package of my choice. memfs is enforced

  static getFs() {
    if (!MoneroWalletFull.FS) MoneroWalletFull.FS = GenUtils.isBrowser() ? memfs.fs.promises : fs.promises;
    return MoneroWalletFull.FS;
  }

This should fix it

  static getFs() {
    return fs.promises;

Full patch which also moves memfs into dev dependencies (needed only for webpack):

diff --git a/package-lock.json b/package-lock.json
index 29161252..e54eba86 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -15,7 +15,6 @@
         "async": "2.6.4",
         "axios": "^1.7.4",
         "crypto-js": "^4.0.0",
-        "memfs": "^4.11.1",
         "net": "^1.0.2",
         "promise-throttle": "^1.1.2",
         "serialize-javascript": "^3.1.0",
@@ -47,6 +46,7 @@
         "eslint-plugin-import": "^2.27.5",
         "eslint-plugin-jsx-a11y": "^6.7.1",
         "https-browserify": "^1.0.0",
+        "memfs": "^4.11.1",
         "mocha": "^9.1.3",
         "os-browserify": "^0.3.0",
         "path-browserify": "^1.0.1",
@@ -1952,6 +1952,7 @@
       "version": "1.1.2",
       "resolved": "https://registry.npmjs.org/@jsonjoy.com/base64/-/base64-1.1.2.tgz",
       "integrity": "sha512-q6XAnWQDIMA3+FTiOYajoYqySkO+JSat0ytXGSuRdq9uXE7o92gzuQwQM14xaCRlBLGq3v5miDGC4vkVTn54xA==",
+      "dev": true,
       "engines": {
         "node": ">=10.0"
       },
@@ -1967,6 +1968,7 @@
       "version": "1.1.0",
       "resolved": "https://registry.npmjs.org/@jsonjoy.com/json-pack/-/json-pack-1.1.0.tgz",
       "integrity": "sha512-zlQONA+msXPPwHWZMKFVS78ewFczIll5lXiVPwFPCZUsrOKdxc2AvxU1HoNBmMRhqDZUR9HkC3UOm+6pME6Xsg==",
+      "dev": true,
       "dependencies": {
         "@jsonjoy.com/base64": "^1.1.1",
         "@jsonjoy.com/util": "^1.1.2",
@@ -1988,6 +1990,7 @@
       "version": "1.3.0",
       "resolved": "https://registry.npmjs.org/@jsonjoy.com/util/-/util-1.3.0.tgz",
       "integrity": "sha512-Cebt4Vk7k1xHy87kHY7KSPLT77A7Ev7IfOblyLZhtYEhrdQ6fX4EoLq3xOQ3O/DRMEh2ok5nyC180E+ABS8Wmw==",
+      "dev": true,
       "engines": {
         "node": ">=10.0"
       },
@@ -5059,6 +5062,7 @@
       "version": "1.2.0",
       "resolved": "https://registry.npmjs.org/hyperdyperid/-/hyperdyperid-1.2.0.tgz",
       "integrity": "sha512-Y93lCzHYgGWdrJ66yIktxiaGULYc6oGiABxhcO5AufBeOyoIdZF7bIfLaOrbM0iGIOXQQgxxRrFEnb+Y6w1n4A==",
+      "dev": true,
       "engines": {
         "node": ">=10.18"
       }
@@ -5881,6 +5885,7 @@
       "version": "4.11.1",
       "resolved": "https://registry.npmjs.org/memfs/-/memfs-4.11.1.tgz",
       "integrity": "sha512-LZcMTBAgqUUKNXZagcZxvXXfgF1bHX7Y7nQ0QyEiNbRJgE29GhgPd8Yna1VQcLlPiHt/5RFJMWYN9Uv/VPNvjQ==",
+      "dev": true,
       "dependencies": {
         "@jsonjoy.com/json-pack": "^1.0.3",
         "@jsonjoy.com/util": "^1.3.0",
@@ -7430,6 +7435,7 @@
       "version": "1.21.0",
       "resolved": "https://registry.npmjs.org/thingies/-/thingies-1.21.0.tgz",
       "integrity": "sha512-hsqsJsFMsV+aD4s3CWKk85ep/3I9XzYV/IXaSouJMYIoDlgyi11cBhsqYe9/geRfB0YIikBQg6raRaM+nIMP9g==",
+      "dev": true,
       "engines": {
         "node": ">=10.18"
       },
@@ -7464,6 +7470,7 @@
       "version": "1.0.2",
       "resolved": "https://registry.npmjs.org/tree-dump/-/tree-dump-1.0.2.tgz",
       "integrity": "sha512-dpev9ABuLWdEubk+cIaI9cHwRNNDjkBBLXTwI4UCUFdQ5xXKqNXoK4FEciw/vxf+NQ7Cb7sGUyeUtORvHIdRXQ==",
+      "dev": true,
       "engines": {
         "node": ">=10.0"
       },
@@ -7665,9 +7672,10 @@
       }
     },
     "node_modules/tslib": {
-      "version": "2.6.3",
-      "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.6.3.tgz",
-      "integrity": "sha512-xNvxJEOUiWPGhUuUdQgAJPKOOJfGnIyKySOc09XkKsgdUV/3E2zvwZYdejjmRgPCgcym1juLH3226yA7sEFJKQ=="
+      "version": "2.7.0",
+      "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.7.0.tgz",
+      "integrity": "sha512-gLXCKdN1/j47AiHiOkJN69hJmcbGTHI0ImLmbYLHykhgeN0jVGola9yVjFgzCUklsZQMW55o+dW7IXv3RCXDzA==",
+      "dev": true
     },
     "node_modules/tsutils": {
       "version": "3.21.0",
diff --git a/package.json b/package.json
index c6310712..19ea60f8 100644
--- a/package.json
+++ b/package.json
@@ -35,7 +35,6 @@
     "async": "2.6.4",
     "axios": "^1.7.4",
     "crypto-js": "^4.0.0",
-    "memfs": "^4.11.1",
     "net": "^1.0.2",
     "promise-throttle": "^1.1.2",
     "serialize-javascript": "^3.1.0",
@@ -67,6 +66,7 @@
     "eslint-plugin-import": "^2.27.5",
     "eslint-plugin-jsx-a11y": "^6.7.1",
     "https-browserify": "^1.0.0",
+    "memfs": "^4.11.1",
     "mocha": "^9.1.3",
     "os-browserify": "^0.3.0",
     "path-browserify": "^1.0.1",
diff --git a/src/main/ts/wallet/MoneroWalletFull.ts b/src/main/ts/wallet/MoneroWalletFull.ts
index 19eb2a38..b926bcb8 100644
--- a/src/main/ts/wallet/MoneroWalletFull.ts
+++ b/src/main/ts/wallet/MoneroWalletFull.ts
@@ -39,7 +39,6 @@ import MoneroMessageSignatureType from "./model/MoneroMessageSignatureType";
 import MoneroMessageSignatureResult from "./model/MoneroMessageSignatureResult";
 import MoneroVersion from "../daemon/model/MoneroVersion";
 import fs from "fs";
-import memfs from "memfs";

 /**
  * Implements a Monero wallet using client-side WebAssembly bindings to monero-project's wallet2 in C++.
@@ -300,8 +299,7 @@ export default class MoneroWalletFull extends MoneroWalletKeys {
   }

   static getFs() {
-    if (!MoneroWalletFull.FS) MoneroWalletFull.FS = GenUtils.isBrowser() ? memfs.fs.promises : fs.promises;
-    return MoneroWalletFull.FS;
+    return fs.promises;
   }

   // ------------ WALLET METHODS SPECIFIC TO WASM IMPLEMENTATION --------------
woodser commented 2 weeks ago

Thanks, please open a PR for the patch.

I recall there was an issue in one of the sample web apps if memfs was not used in this way, so the sample apps should be tested too.

woodser commented 2 weeks ago

I've removed memfs from the full wallet and moved to dev dependencies: https://github.com/woodser/monero-ts/pull/243/commits/ed1aba7f3ee3690fa027f5bd6523f79ca0f10f16

I tested on the sample apps and don't see a problem with memfs.