Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for typescript via type definitions in @types folder. #93

Closed
wants to merge 7 commits into from

Conversation

BeanTosser
Copy link
Contributor

Add support for typescript via type definitions in @types folder.

Install typescript, emit typedefs, and "fix" index.d.ts.

Replace all BigInteger math function calls with standard BigInt...

... overloads (BigInteger(value).multiply(BigInteger(value2)) ->
BigInt(value) * BigInt(value2))

Type nearly everything in TestSampleCode.ts.

Fix all errors that were due to typing vars in "for...of" statements.

NOTE:
keyImage is defined as string. This is probably correct but can't be
verified due to lack of documentation and my inability to trace the
method chain in the source code.

Intermediate progress writing script to replace BigInteger.compare.

Find lefthand terms of .compare operations.

Replacing BigInteger.compare() with BigIntegerCompare() in progress.

Fix all errors and finish converting TestSampleCode.js to Typescript.

Update "async" to version 2.6.4.

@BeanTosser
Copy link
Contributor Author

This was a messy, messy merge/rebase. Please take a good look at the diffs to make sure nothing screwy is going on.

Copy link
Owner

@woodser woodser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very happy to be getting a PR to support TypeScript :)

See comments throughout my review.

The WASM builds, but I'm getting the following error when ./bin/build_web_worker.sh runs. Rather than debugging this issue, you might just circumvent it by moving to a static utility, GenUtils.compareBigInt().

ERROR in ./src/main/js/common/GenUtils.js 3:26-71
Module not found: Error: Can't resolve '~/main/js/common/BigIntegerCompare' in '/Users/woodser/git/monero-javascript/src/main/js/common'
resolve '~/main/js/common/BigIntegerCompare' in '/Users/woodser/git/monero-javascript/src/main/js/common'
  Parsed request is a module
  using description file: /Users/woodser/git/monero-javascript/package.json (relative path: ./src/main/js/common)
    aliased with mapping '~': '/Users/woodser/git/monero-javascript/app' to '/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare'
      using description file: /Users/woodser/git/monero-javascript/package.json (relative path: ./src/main/js/common)
        Field 'browser' doesn't contain a valid alias configuration
        root path /Users/woodser/git/monero-javascript
          using description file: /Users/woodser/git/monero-javascript/package.json (relative path: ./Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.js doesn't exist
            .jsx
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.jsx doesn't exist
            .css
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.css doesn't exist
            .json
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.json doesn't exist
            otf
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompareotf doesn't exist
            ttf
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerComparettf doesn't exist
            eot
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompareeot doesn't exist
            svg
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerComparesvg doesn't exist
            .ts
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.ts doesn't exist
            .tsx
              Field 'browser' doesn't contain a valid alias configuration
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.tsx doesn't exist
            as directory
              /Users/woodser/git/monero-javascript/Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare doesn't exist
        using description file: /Users/woodser/git/monero-javascript/package.json (relative path: ./app/main/js/common/BigIntegerCompare)
          no extension
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare doesn't exist
          .js
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.js doesn't exist
          .jsx
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.jsx doesn't exist
          .css
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.css doesn't exist
          .json
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.json doesn't exist
          otf
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompareotf doesn't exist
          ttf
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerComparettf doesn't exist
          eot
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompareeot doesn't exist
          svg
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerComparesvg doesn't exist
          .ts
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.ts doesn't exist
          .tsx
            Field 'browser' doesn't contain a valid alias configuration
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare.tsx doesn't exist
          as directory
            /Users/woodser/git/monero-javascript/app/main/js/common/BigIntegerCompare doesn't exist
 @ ./src/main/js/common/MoneroWebWorker.js 3:17-38

.gitignore Outdated Show resolved Hide resolved
@@ -0,0 +1,341 @@
#include <emscripten.h>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why creating a new file named #http_client_wasm.cpp#? Probably this file doesn't need changed at all?

Copy link
Contributor Author

@BeanTosser BeanTosser Jun 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Erm... your guess is as good as mine, actually. I definitely did not manually or intentionally create this file. Presumably it was created when I was building the monero-javascript source?

src/main/js/common/BigIntegerCompare.js Outdated Show resolved Hide resolved
src/main/js/common/LibraryUtils.js Outdated Show resolved Hide resolved
src/main/js/common/LibraryUtils.js Outdated Show resolved Hide resolved
src/test/TestMoneroWalletRpc.js Outdated Show resolved Hide resolved
src/test/TestSampleCode.ts Outdated Show resolved Hide resolved
src/test/TestSampleCode.ts Outdated Show resolved Hide resolved
src/test/TestSampleCode.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,5 @@
export = monero_javascript;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

types/src and types/dist exist. I'll need to understand how to generate the source types and what modifications are necessary. I assume files within dist are 100% auto generated.

Copy link
Contributor Author

@BeanTosser BeanTosser Jun 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To generate the initial typedefs, use the following command:

npx -p typescript tsc src/*.js --declaration --allowJs --emitDeclarationOnly --outDir types

After that each entry in index.d.ts needs to be changed to match the format:

  import * as MoneroWalletKeys from "./src/main/js/wallet/MoneroWalletKeys";
  export import MoneroWalletKeys = MoneroWalletKeys;

...with the exception of functions, which should look like:

  export function getVersion(): string;
  export function connectToDaemonRpc(...args: any[]): MoneroDaemonRpc;
  export function connectToWalletRpc(...args: any[]): MoneroWalletRpc;
  export function createWalletFull(...args: any[]): MoneroWalletFull;
  export function openWalletFull(...args: any[]): MoneroWalletFull;
  export function createWalletKeys(...args: any[]): MoneroWalletKeys;

There may be more changes necessary in other files in types/src... I can't recall for sure off the top of my head. We will need to generate a new set of typedifs and get the diffs comparing my types folder with the freshly generated folder to see what other manual changes might be needed.

The types/dist folder may actually be completely unnecessary. We'd have to test it with the folder removed to be sure.

@woodser
Copy link
Owner

woodser commented Jun 6, 2022

Related issue: #81

@BeanTosser BeanTosser marked this pull request as draft June 7, 2022 00:28
types/index.d.ts Outdated
export import MoneroConnectionManager = MoneroConnectionManager;
import * as MoneroConnectionManagerListener from "./src/main/js/common/MoneroConnectionManagerListener";
export import MoneroConnectionManagerListener = MoneroConnectionManagerListener;
import * as MoneroUtils from "./src/main/js/common/MoneroUtils";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the import and export lines for all of these things could be replaced with something like:
export * as MoneroUtils from "monero-javascript/types/src/main/js/common/MoneroUtils"; . The index file is pulling all of these declaration files into one place and re-exporting; Note that I've replaced the actual JS location with the type location- this is important. Try importing your branch into a typescript project and call const utils = monerojs.MoneroUtils and you'll see that the definitions and methods now pop up.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, this index.d.ts file location should (I think) be referenced in the monero-javascript package.json as "types": "types/index.d.ts"

* @param {string} privateViewKey is the private view key to validate
* @return {bool} true if the private view key is valid, false otherwise
*/
static isValidPrivateViewKey(privateViewKey: string): bool;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be boolean. Maybe do a find and replace on the whole types folder.

@CryptoGrampy
Copy link

Wow. This is a crazy amount of code... it will be extremely useful for anyone using MoneroJs in the future. Great job. I was able to pull your PR into my active project by replacing monero-javascript in the package.json file with "monero-javascript": "BeanTosser/monero-javascript#add_typescript_support" (if anyone's interested)

It may be worth breaking things into additional index.d.ts files in each group (common, daemon, etc) just to make things a little less heavy in the big index file (though you would have to import the group indexes into the main index 🙃 )

Bugs:

I'm not able to run this yet- first bug I'm seeing: Module not found: Error: Can't resolve './src/main/js/common/BigIntegerCompare' in 'node_modules/monero-javascript'

@woodser woodser mentioned this pull request Oct 14, 2022
@woodser
Copy link
Owner

woodser commented Oct 14, 2022

Replacing with #108

@woodser woodser closed this Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants