aeternity / aepp-sdk-js

JavaScript SDK for the æternity blockchain
http://docs.aeternity.com/aepp-sdk-js/
ISC License
120 stars 59 forks source link

TS errors on build process #1671

Closed nikita-fuchs closed 1 year ago

nikita-fuchs commented 2 years ago

I get TS errors when the SDK is transpiled, which don't show up in the Angular boilerplate

Example:

Error: node_modules/@aeternity/aepp-sdk/es/AeSdk.d.ts:6:10 - error TS1023: An index signature parameter type must be either 'string' or 'number'.

6         [key: Encoded.AccountAddress]: AccountBase;
           ~~~

Error: node_modules/@aeternity/aepp-sdk/es/AeSdkBase.d.ts:18:25 - error TS2304: Cannot find name 'Awaited'.

18 declare type NodeInfo = Awaited<ReturnType<Node['getNodeInfo']>> & {
                           ~~~~~~~

Error: node_modules/@aeternity/aepp-sdk/es/AeSdkBase.d.ts:140:58 - error TS2314: Generic type 'Omit' requires 2 type argument(s).

140     buildTx<TxType extends Tag>(txType: TxType, options: Omit<Parameters<typeof _buildTx<TxType>>[1], 'onNode'> & {
                                                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

As I'm not a TS pro: Does it have to do with the TS version "typescript": "^4.2.3" used in the UI template of my project? Unfortunately, I can't update it easily. The boilerplate has "typescript": "~4.7.3". I performed the required additions to tsconfig. Could someone please give it a simple try, this branch has the SDK installed already:

https://github.com/nikita-fuchs/nftminter/tree/aesdk_installed

Thanks a lot for your help !

davidyuk commented 2 years ago

The problem is in TS version that you are using

error TS2304: Cannot find name 'Awaited'

TypeScript 4.5 introduces a new utility type called the Awaited type

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-5.html#the-awaited-type-and-promise-improvements

As a workaround, I can make a version of sdk without types or do this on your side: https://github.com/microsoft/TypeScript/issues/17042#issuecomment-317589758

davidyuk commented 2 years ago

With this changes:

diff --git a/src/sdk-mock.d.ts b/src/sdk-mock.d.ts
new file mode 100644
index 0000000..48f2f70
--- /dev/null
+++ b/src/sdk-mock.d.ts
@@ -0,0 +1 @@
+declare module '@aeternity/aepp-sdk';
diff --git a/src/tsconfig.app.json b/src/tsconfig.app.json
index ba21981..02cd373 100644
--- a/src/tsconfig.app.json
+++ b/src/tsconfig.app.json
@@ -2,7 +2,10 @@
   "extends": "../tsconfig.json",
   "compilerOptions": {
     "outDir": "../out-tsc/app",
-    "baseUrl": "./"
+    "baseUrl": "./",
+    "paths": {
+      "@aeternity/aepp-sdk": ["./sdk-mock.d.ts"]
+    }
   },
   "files": [
     "main.ts",

there is only one error:

Build at: 2022-09-01T15:31:32.214Z - Hash: ecbd78387f7fb027752a - Time: 98559ms

Error: src/app/services/aeternity.service.ts:17:11 - error TS2709: Cannot use namespace 'AeSdkAepp' as a type.

17   aeSdk?: AeSdkAepp;
             ~~~~~~~~~

you can replace AeSdkAepp with any and so on 🤷‍♀️

marc0olo commented 2 years ago

Could someone please give it a simple try, this branch has the SDK installed already:

https://github.com/nikita-fuchs/nftminter/tree/aesdk_installed

what is this example built upon? just wondering as I don't see where it is forked from

I get TS errors when the SDK is transpiled, which don't show up in the Angular boilerplate

why aren't you using the boilerplate which we recently updated? (where you don't have the problem like you stated)

nikita-fuchs commented 2 years ago

@marc0olo it's built upon https://github.com/akveo/ngx-admin , I didn't use the boilerplate as I assumed it's easier to implement the SDK somewhere than implementing the rest into something where the SDK already works. While this assumption should generally accurate, typescript introduced a new compatibility hurdle.

@davidyuk I will try my best to understand those hacks. generally speaking though: To allow for easier backwards compatibility, could we also release a non-TS version of the SDK? Sometimes for all sorts of silly dependency reasons the TS version can't be updated, like in my case, and then you're stuck without options.

nikita-fuchs commented 2 years ago

@davidyuk While building now works, I'm running into this issue at runtime when initialising the sdk:

core.js:6456 ERROR Error: Uncaught (in promise): TypeError: _aeternity_aepp_sdk__WEBPACK_IMPORTED_MODULE_0__.Node is not a constructor
TypeError: _aeternity_aepp_sdk__WEBPACK_IMPORTED_MODULE_0__.Node is not a constructor

which refers to this line of code: https://github.com/nikita-fuchs/nftminter/blob/aesdk_installed/src/app/services/aeternity.service.ts#L27

davidyuk commented 2 years ago

For some reason, the previously proposed workaround completely remove sdk contents 🤷‍♀️

As another workaround, I'm proposing to use commonjs version of SDK: https://github.com/nikita-fuchs/nftminter/pull/1/files Though it will be forbidden in the next release of SDK (https://github.com/aeternity/aepp-sdk-js/pull/1659).

could we also release a non-TS version of the SDK?

I don't like this idea, should be possible to make ts to ignore definitions of specific library, but I can't find it yet.

marc0olo commented 2 years ago

independent of backwards compatibility I am not a fan of providing example applications which are:

we should provide examples that are easy to follow and if possible using the latest versions of libraries if there is no specific reason why we use some older version.

could we also release a non-TS version of the SDK?

agree with @davidyuk, not a fan of going this route.

should be possible to make ts to ignore definitions of specific library, but I can't find it yet.

if you find a solution to that please add this to the documentation of the SDK :-)

nikita-fuchs commented 2 years ago

not a fan of going this route. should be possible to make ts to ignore definitions of specific library, but I can't find it yet.

I'm not opinionated about the route, as long as the solution is that TS gets out of the way doing the basic things. In order to maximise the compatibility of the SDK to as many projects as possible, could you consider downleveling the types for other TS versions, like here @davidyuk ? If I understand correctly, this would solve the problem here?

marc0olo commented 2 years ago

downleveling the types for other TS versions, like here

that sounds interesting - but not sure about the effort and if it's worth it 🤔

btw we should update the compatibility matrix and typescript info I guess @davidyuk

davidyuk commented 1 year ago

https://github.com/nikita-fuchs/nftminter/pull/2 It looks working now, at least in the console ✔ Compiled successfully.

nikita-fuchs commented 1 year ago

@davidyuk I should have mentioned in here that I've got it working by updating to Angular >=14 already, so no need for adjustments at least in my case, but from what I heard from @subhod-i there are Vue projects struggling from the high TS version?

davidyuk commented 1 year ago

the high TS version

actually, not a big difference, they have 4.3, and you had 4.2

nikita-fuchs commented 1 year ago

It's working now for me, I've upgraded the nft minter to fresh new Angular 15 with TS 4.8.4. thanks for your effort though !

I think we can close this issue if the TS version needed for the SDK is added to the requirements sheet. Wich one is it now ? 4.7.4 or did you downgrade it for the next release?

Where is that min. version requirements doc gone, btw? I've PRd my findings about angular but can't find it now anymore. We need to put it into the quickstart and installation instructions of the SDK repo I think?

davidyuk commented 1 year ago

Wich one is it now?

Ideally 4.7, but sdk would support 4.1 with less precise types

We need to put it into the quickstart and installation instructions of the SDK repo I think?

do you mean https://github.com/aeternity/aepp-sdk-js/issues/1703? I will check it a bit later