woodser / monero-ts

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

Error importing monero-ts since updating Axios #235

Closed woodser closed 2 weeks ago

woodser commented 3 weeks ago

Since the Axios update in v0.10.1, I'm hitting an error using monero-ts in haveno-ts:

 FAIL  src/HavenoClient.test.ts
  ● Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/en/ecmascript-modules for how to enable it.
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /Users/woodser/git/haveno-ts/node_modules/axios/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import axios from './lib/axios.js';
                                                                                             ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      51 | import assert from "assert";
      52 | import console from "console"; // import console because jest swallows messages in real time
    > 53 | import moneroTs from "monero-ts";
         | ^
      54 | import * as os from 'os';
      55 |
      56 | // ------------------------------ TEST CONFIG ---------------------------------

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1350:14)
      at Object.<anonymous> (node_modules/monero-ts/src/main/ts/common/HttpClient.ts:7:1)
      at Object.<anonymous> (node_modules/monero-ts/index.ts:11:1)
      at Object.<anonymous> (src/HavenoClient.test.ts:53:1)
woodser commented 3 weeks ago

@mainnet-pat Hoping you can help fix this? It doesn't like the import axios statement for some reason. I'd like to fix the root problem within monero-ts and not require any changes to downstream consumers, if possible.

woodser commented 3 weeks ago

Relevant stackoverflow: https://stackoverflow.com/questions/73958968/cannot-use-import-statement-outside-a-module-with-axios

mainnet-pat commented 3 weeks ago

How can I reproduce this locally?

mainnet-pat commented 3 weeks ago

My other projects never showed these erorrs

woodser commented 3 weeks ago

How can I reproduce this locally?

By running any of the tests in the haveno-ts project: https://github.com/haveno-dex/haveno-ts?tab=readme-ov-file#run-tests

woodser commented 3 weeks ago

My other projects never showed these erorrs

Yeah I didn't see it in any of the sample apps. It was my mistake to not test it in haveno-ts prior to release.

woodser commented 3 weeks ago

According to the stackoverflow, the issue might resolve by updating to the latest version of jest, but that introduces its own build issue (ReferenceError: proto is not defined).

mainnet-pat commented 3 weeks ago

Cant look too deep into this. Packing the bags for a weekend trip.

It seemed to help me to install the following (latest versions):

npm i --save-dev jest @types/jest ts-jest ts-jest-resolver

remove your config from package.json and add a jest.config.cjs from my other project:

module.exports = {
  rootDir: "./",
  preset: "ts-jest/presets/default-esm",
  resolver: "ts-jest-resolver",
  collectCoverage: true,
  collectCoverageFrom: [
    "**/*.{js,jsx,ts}",
    "!**/node_modules/**",
  ],
  coveragePathIgnorePatterns: [
    ".*/src/.*\\.d\\.ts",
    ".*/src/.*\\.test\\.{ts,js}",
    ".*/src/.*\\.test\\.headless\\.js",
  ],
  roots: [
    "<rootDir>/src",
  ],
  testMatch: [
    "**/__tests__/**/*.+(ts|tsx|js)",
    "**/?(*.)+(spec|test).+(ts|tsx|js)",
  ],
  transform: {
    "^.+\\.tsx?$": [
      "ts-jest",
      {
        useESM: true,
      },
    ],
  },
  testEnvironment: "node",
  verbose: true,
  maxConcurrency: 1,
  testTimeout: 180000,
  testPathIgnorePatterns: [
    "/node_modules/",
    "/dist/",
  ],
  globals: {
    window: {}
  },
};
woodser commented 3 weeks ago

remove your config from package.json and add a jest.config.cjs from my other project:

Doing so leads to errors like these:

 FAIL  src/HavenoClient.test.ts
  ● Test suite failed to run

    src/HavenoClient.test.ts:94:3 - error TS2564: Property 'havenod' has no initializer and is not definitely assigned in the constructor.

    94   havenod: HavenoClient;
         ~~~~~~~
    src/HavenoClient.test.ts:95:3 - error TS2564: Property 'wallet' has no initializer and is not definitely assigned in the constructor.

    95   wallet: moneroTs.MoneroWallet;
         ~~~~~~
    src/HavenoClient.test.ts:96:3 - error TS2564: Property 'trade' has no initializer and is not definitely assigned in the constructor.

    96   trade: TradeInfo;
mainnet-pat commented 3 weeks ago

Looks like linter stuff, so should be fixable

mainnet-pat commented 3 weeks ago

for example like this:

class PeerContext {
  havenod!: HavenoClient;
  wallet!: moneroTs.MoneroWallet;
  trade!: TradeInfo;
....
woodser commented 3 weeks ago

Fixing the lint errors gets closer, but now getting: XMLHttpRequest is not defined.

mainnet-pat commented 3 weeks ago

it is not defined for node

woodser commented 3 weeks ago

Got further by changing testEnvironment: "node" to testEnvironment: "jsdom", but now getting cors errors because requests originate from "http://localhost" instead of "http://localhost:8080".

woodser commented 3 weeks ago

Adding "testURL": "http://localhost:8080" changes the error to: 401 Unauthorized: <html><head><title>Unauthorized Access</title></head><body><h1>401 Unauthorized</h1></body></html>

As of now I'm still not able to use the new monero-ts update in the haveno-ts library.

woodser commented 3 weeks ago

Adding "testURL": "http://localhost:8080/" changes the error to: 401 Unauthorized: Unauthorized Access

401 Unauthorized

I'm wondering if it's failing on the digest authentication challenge in this environment.

mainnet-pat commented 3 weeks ago

This seems like the response from the daemon, why would jest render this?

mainnet-pat commented 3 weeks ago

I can not compile the protobuf stuff atm. so can not test exactly your code

mainnet-pat commented 3 weeks ago

Built haveno and protobuf but fail with TypeError: Jest: a transform must export aprocessfunction.

I run out of time, will be back on Monday

woodser commented 2 weeks ago

Here is the error in the CI tests if it helps: https://github.com/haveno-dex/haveno-ts/actions/runs/10564898796/job/29268294819

mainnet-pat commented 2 weeks ago

I've spent 3 more hours on this, I still can not set up the haveno envronment locally, it is simply too complex. If monero-ts returns you actual 401 response with some body, this means that axios http client works, so it must be something with your wallet-rpc service. I think fixing haveno-ts for you is beyond the scope of my involvement here unless you will offer me a bounty for this

I have tried monero-ts in other projects using up-to-date jest for tests, and new http client adaptor based on axios works for me there.

woodser commented 2 weeks ago

I can offer a bounty of 2 additional XMR to fix using the library in haveno-ts. As it is, I'm stuck. I'd be happy to DM to help set up the haveno-ts environment so we don't lose time there.

mainnet-pat commented 2 weeks ago

Okay, I take this bounty offer and will take another look at haveno-ts.

My main problem is that your docker setup is using host networking which does not work in my macOS environment. Firstly I want to migrate the docker compose setup to use docker internal networking, then it will be better portable and testable

woodser commented 2 weeks ago

your docker setup is using host networking

Ah! I was just playing with this yesterday, and realized that on macOS, it will not work if you include --network host in the command to start envoy.

So this needs removed and it should work as is.

mainnet-pat commented 2 weeks ago

this would not be enough, proper ports shall be exposed and services linked by name like --daemon-address=node1:28081 instead of by ip like --daemon-address=127.0.0.1:28081

woodser commented 2 weeks ago

I'd highly recommend getting the project running as is before updating things (which would be great). Again, happy to help get that first step working over DM or here or whatever.

mainnet-pat commented 2 weeks ago

the best I can do to make it run as is is to test on some linux vps (I assume you run linux, at least github actions are linux based)

woodser commented 2 weeks ago

I'm running macOS, and all tests pass for me on that environment.

woodser commented 2 weeks ago

It also requires an old version of Docker.

mainnet-pat commented 2 weeks ago

well, idk about downgrading my docker, but if you mean it, I will do

woodser commented 2 weeks ago

Yes it will definitely not work with the latest version of docker. I lost the better part of yesterday wrestling with this. Downgrading back to Docker 4.26.1 worked like a charm.

woodser commented 2 weeks ago

This issue is resolved in the 0.10.3 release.