js-stellar-wallets
js-stellar-wallets copied to clipboard
deps: upgrade some dependencies, including TypeScript and trezor-connect etc.
This improvement is mainly contributed by @mkalen.
Hi @quietbits, can you help me see why it doesn't compile successfully?
Looks like it's caused by an outdated version of TypeScript, maybe we should upgrade it to the latest version(v4+).
@overcat, I don't see any issues upgrading TypeScript to v4. There might be other packages we'll need to update to support the newer TS version, which might turn into a bigger update.
I forked the overcat fork and made a suggested TS4 upgrade, see https://github.com/mkalen/js-stellar-wallets/commit/a280f6eb8cae60458b7d1a1756b094ea0016892f.
It TypeScript-compiles with TS4. Also builds on Windows. All tests pass.
There is one remaining incompatibility between Trezor Connect's Stellar Plug-In and this package, the transaction Memo type does not match completely. I can get it to TS-compile by changing line 17 of node_modules/@trezor/connect-plugin-stellar/lib/index.d.ts from id: string | Buffer | null; to id: string | undefined;.
Obviously that last bit needs to be sorted before CI testing can pass. Maybe Trezor e.g. @Hannsek can help you out with the above? Thanks.
I forked the overcat fork and made a suggested TS4 upgrade, see mkalen@a280f6e. It TypeScript-compiles with TS4. Also builds on Windows. All tests pass. There is one remaining incompatibility between Trezor Connect's Stellar Plug-In and this package, the transaction Memo type does not match completely. I can get it to TS-compile by changing line 17 of node_modules/@trezor/connect-plugin-stellar/lib/index.d.ts from
id: string | Buffer | null;toid: string | undefined;. Obviously that last bit needs to be sorted before CI testing can pass. Maybe Trezor e.g. @Hannsek can help you out with the above? Thanks.
Fixed in https://github.com/trezor/trezor-suite/pull/7395
Hi @mkalen, can you check this? https://github.com/trezor/trezor-suite/pull/7451#issuecomment-1403747403
Hi, @quietbits, can you make the Netlify deployment log public?
https://www.netlify.com/blog/2017/10/31/introducing-public-deploy-logs-for-open-source-sites/
Hi, @quietbits, can you make the Netlify deployment log public?
https://www.netlify.com/blog/2017/10/31/introducing-public-deploy-logs-for-open-source-sites/
We're looking into making the logs public.
Looks the issue was the Yarn version being outdated. I can upgrade the version the env uses a little bit later today and see if that fixes it:
7:21:56 PM: error [email protected]: The engine "yarn" is incompatible with this module. Expected version "^1.15.2". Got "1.13.0"
7:21:56 PM: error Found incompatible module
@overcat I fixed the issues with the environment, but looks like you have some TS issues with the code in your PR. This is the build command Netlify runs:
yarn docs && cd ./documentation && yarn install && yarn build
When you run that on your code, you will notice you get some typedoc errors. I think once you fix those, you should be good to go
@overcat There is now a suggested expected completion for this PR: https://github.com/overcat/js-stellar-wallets/pull/2 - builds cleanly locally using Netlify commands. Local generated documentation site is comparable to current https://stellar-walletsdk-docs.netlify.app (Since the TS4 transition makes scope go beyond a pure Trezor Connect v9 upgrade, it's probably recommended to also bump the js-stellar-wallets version on semver y...)
Hi @mkalen, I have invited you to be a co-author of overcat/js-stellar-wallets, and you can edit this PR directly after accepting the invitation.
This improvement is mainly contributed by @mkalen.
Don't know about that. :-) I consider the TS4 stuff somewhat of a "side-effect" of the main goal for a Trezor Connect v9 upgrade - contributed 100% by @overcat.
Unsure why Netlify build still fails, tricky when the logs are not public. And I can't see where the CI test cases are defined, is that available in this Git repo? Stuff will move around a bit on the documentation site ToC, but I have compared that exported types are visible and rendered in a similar fashion as current published Netlify site. It's not 1:1 between TS3 and TS4 though, hence the recommended package version bump.
Building to a fresh Netlify site works after last sh syntax fix/regression revert on doc site generation. So I need a hint from the actual Netlify logs @piyalbasu. Thanks.
Netlify env that worked used:
"Now using node v16.19.0 (npm v8.19.3)
(...)
Installing npm packages using Yarn version 1.22.19
(...)
@netlify/build 29.5.1"
Resulting site: https://classy-druid-e01dfc.netlify.app/
Hi all - sorry for how annoying this has been to debug to with private logs. The issue was an outdated Node version. If you push an empty commit, this should re-trigger the build and it should pass now
I'm also creating a ticket to make these logs public. We need to do a security audit before we can enable this, which is why we haven't been able to do this yet. Thanks for your patience!
Hi all - sorry for how annoying this has been to debug to with private logs. The issue was an outdated Node version. If you push an empty commit, this should re-trigger the build and it should pass now
Thanks. No worries for my part - I've learned a lot about Stellar Wallet SDK during the process! Great that the Netlify blocked CI build root cause was found, I will make a commit to trigger a build.
Apart from Trezor Connect v9 upgrade, the TS4 + TypeDoc upgrade of this PR addresses networkPassphrase missing from README (#195) - see https://deploy-preview-333--stellar-walletsdk-docs.netlify.app/.