network icon indicating copy to clipboard operation
network copied to clipboard

refactor: Convert `bin` scripts to TypeScript

Open teogeb opened this issue 1 year ago • 1 comments

TODO Why some changes needed for Storage.ts, killing-dead-connections.test.ts, SubscriptionSession.ts. Without the changes we get eslint/build errors. Most likely not related to this PR, and maybe these fail in main, too? Changes to these files should be in a separate PR?

Convert all bin scripts to TypeScript:

  • tracker
  • broker, broker-init and delete-expired-data

No functionality changes except that tracker.ts now uses parseInt to parse integer command line arguments.

Execution permission

Also removed the execution permission from these .ts files as we never execute the TypeScript files (only JavaScript files compiled from the files).

We could set the execution permission for generated .js files if needed. But it seems that the permission is not required for end-users: e.g. npm install -g @streamr/cli-tools installs cli-tools ok without the permissions. Therefore we don't add unnecessary complexity to the build process in this PR.

The permission is most likely needed for development scripts which we run via IDE, e.g. .idea/runConfigurations/*.xml, but we can set permission manually if needed.

Changes in run-brokers-and-trackers script

Refactored the script to use broker.js and tracker.js directly instead of using npm exec. It would be better to use npm exec (as we wouldn't need to hardcode the name of the JS file). But it seems that sometimes Broker startup fails with the following error if we use npm exec: "npm WARN exec The following package was not found and will be installed: streamr-broker"

Other changes

  • Added private key argument for Tracker in Dockerfile.tracker: the private key is required, and with this fix the Tracker starts ok also without a custom command (which we always provide when using streamr-docker-dev)
  • Removed benchmark test which depended on previously removed subscriber.js bin script (https://github.com/streamr-dev/network/pull/1029)

Future improvements

  • Fix several code lines which have annotated with @ts-expect-error
  • Could modify config-wizard and delete-expired-data so that the main code is run via .action callback, similarly to bin/broker.ts
  • Convert @streamr/slackbot to TypeScript?
  • Could update npm? During startup we get notice: npm notice New major version of npm available! 8.19.2 -> 9.1.3
    • Could possibly suppress the message. But it seems that e.g. this doesn't suppress it: CMD ["/usr/local/bin/npm", "--quiet", "exec", "streamr-tracker", "--", "0x0000000000000000000000000000000000000000000000000000000000000001"]

teogeb avatar Dec 01 '22 11:12 teogeb

TODO Why some changes needed for Storage.ts, killing-dead-connections.test.ts, SubscriptionSession.ts. Without the changes we get eslint/build errors. Most likely not related to this PR, and maybe these fail in main, too? Changes to these files should be in a separate PR?

Maybe double-check that package-lock.json is in sync?

harbu avatar Dec 09 '22 11:12 harbu