Add typescript support for aepp-sdk #1113

the-icarus · 2021-02-19T17:19:52Z

I added basic typescript support for the aepp-sdk so we are able to code in typescript and also users of the sdk get type completion in typescript.

This is an initial pull request which already contains transformation where I already converted utils/account-formatter.js and utils/bignumber.js to typescript.

There are still open discussions how we want to build the SDK. With the current setup javascript users might only able to use the build assets from dist so I guess we still need to adjust the build process but this is something I want to discuss with the current developers/maintainers of the SDK.

Happy to receive your feedback.

…e json to enable type detection.

the-icarus · 2021-02-22T16:57:58Z

Hi @davidyuk ,
I changed the PR to be a draft.
There are still some things to discuss and to change. I updated the build so during the npm pack and npm publish phase it will run tsc to process everything by the typescript compiler and get type definition files for all source files. Those are added to the dist/ folder.
I also want to change the package structure. Currently, everything is contained in a release. In my option only the build sources (everything in dist/) should be part of a release. This will exclude stuff like tests, raw docs,... from release which will lead to a clean npm package. please share your thoughts on this. As a first step we can compile and publish in a way that we don't break the implementation of our users.

the-icarus · 2021-03-01T16:33:49Z

@davidyuk I made a final change and customized the npm pack so the generated code .js + .d.ts files are at the same location as the original code. This will allow us to add typescript without introducing breaking changes to our users.

Happy to receive your feedback!

the-icarus · 2021-03-04T12:10:44Z

@davidyuk Any update regarding the review?

omar-saadoun · 2021-03-12T17:16:04Z

Hi @davidyuk please check this

davidyuk

All these notices are resolved in #1131. @the-icarus please take a look at that PR.

.babelrc

davidyuk · 2021-03-25T23:36:25Z

.gitignore

@@ -21,3 +21,4 @@ coverage.*
 /examples/**/dist
 .history
 .site
+*.tgz


npm pack is an edge case, I don't think that ignore files shouldn't be aware of it

davidyuk · 2021-03-25T23:41:49Z

.npmignore

@@ -0,0 +1,2 @@
+es_src


.npmignore overrides .gitignore, currently it brings extra files to the packed package, for example, I got a local package about 40mb because of node_modules of examples. So, if .npmignore is used, it should contain all records from .gitignore. Alternatively, I'm proposing to use a files property of package.json to add only necessary folders (dist and es).

davidyuk · 2021-03-25T23:43:05Z

es/utils/amount-formatter.ts

@@ -56,7 +56,7 @@ export const DENOMINATION_MAGNITUDE = {
 * @param {String} [options.denomination='aettos'] denomination of amount, can be ['ae', 'aettos']
 * @return {String}
 */
-export const toAe = (value, { denomination = AE_AMOUNT_FORMATS.AETTOS } = {}) => formatAmount(value, { denomination, targetDenomination: AE_AMOUNT_FORMATS.AE })
+export const toAe = (value: string | number | BigNumber, { denomination = AE_AMOUNT_FORMATS.AETTOS } = {}) => formatAmount(value, { denomination, targetDenomination: AE_AMOUNT_FORMATS.AE })


this line is too long, I'm planning to limit it by linter in future

davidyuk · 2021-03-25T23:44:42Z

es/utils/bignumber.ts

@@ -3,42 +3,32 @@
 * @module @aeternity/aepp-sdk/es/utils/bignumber
 * @example import { parseBigNumber, asBigNumber, isBigNumber, ceil } from '@aeternity/aepp-sdk/es/utils/bignumber'
 */
-import BigNumber from 'bignumber.js'
+import BigNumber from 'bignumber.js';


; is not allowed by our linter rules, need to switch from standard to ts-standard for linting

davidyuk · 2021-03-25T23:46:07Z

package.json

    "prepare": "npm run build",
    "genChangelog": "conventional-changelog -p angular -i CHANGELOG.md -s",
-    "prepublishOnly": "npm run docs:docco && npm run docs:api"
+    "prepublishOnly": "npm run docs:docco && npm run docs:api",
+    "prepack": "tsc && node tooling/prepack.js",


proposing to move tsc to build script

davidyuk · 2021-03-25T23:47:14Z

package.json

@@ -2,7 +2,8 @@
  "name": "@aeternity/aepp-sdk",
  "version": "7.7.0",
  "description": "SDK for the æternity blockchain",
-  "main": "dist/aepp-sdk.js",
+  "main": "dist/index.js",


this file is es/index.js, as I understand this change needs to be reverted

davidyuk · 2021-03-25T23:48:46Z

package.json

    "@babel/register": "7.10.1",
    "@babel/runtime": "^7.10.2",
+    "@types/bignumber.js": "^5.0.0",


proposing to remove this package because of warning:

npm WARN deprecated @types/bignumber.js@5.0.0: This is a stub types definition for bignumber.js (https://github.com/MikeMcl/bignumber.js/). bignumber.js provides its own type definitions, so you don't need @types/bignumber.js installed!

davidyuk · 2021-03-25T23:50:46Z

tsconfig.json

+    "outDir": "./dist/es",
+    "noImplicitAny": true,
+    "module": "es6",
+    "target": "es2017",


switching it to es5 should solve #196

davidyuk · 2021-03-25T23:52:41Z

tooling/prepack.js

@@ -0,0 +1,5 @@
+const fs = require('fs-extra')
+
+fs.move('es', 'es_src')


Proposing a more radical change: to rename current es to src (as it should be) and to build typescript straight to /es

davidyuk · 2021-04-20T16:14:23Z

refactored version merged in #1131

Add typescript support for aepp-sdk

17d1bef

davidyuk self-requested a review February 22, 2021 07:03

Add typescript compile before package and publish. Add type to packag…

716941c

…e json to enable type detection.

the-icarus marked this pull request as draft February 22, 2021 16:52

add custom pre- and postpack to add type definitions to final asset

5c3ef5d

the-icarus marked this pull request as ready for review March 1, 2021 16:32

Fix lint

cafc752

the-icarus mentioned this pull request Mar 4, 2021

Use jsdoc convetions to enable autocompletion for editors #736

Closed

3 tasks

davidyuk mentioned this pull request Mar 25, 2021

Add typescript support #1131

Merged

davidyuk reviewed Mar 25, 2021

View reviewed changes

davidyuk closed this Apr 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typescript support for aepp-sdk #1113

Add typescript support for aepp-sdk #1113

the-icarus commented Feb 19, 2021

the-icarus commented Feb 22, 2021 •

edited

Loading

the-icarus commented Mar 1, 2021

the-icarus commented Mar 4, 2021

omar-saadoun commented Mar 12, 2021

davidyuk left a comment

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

davidyuk Mar 25, 2021

nikita-fuchs Mar 26, 2021

davidyuk commented Apr 20, 2021

		@@ -0,0 +1,5 @@
		const fs = require('fs-extra')

		fs.move('es', 'es_src')

Add typescript support for aepp-sdk #1113

Add typescript support for aepp-sdk #1113

Conversation

the-icarus commented Feb 19, 2021

the-icarus commented Feb 22, 2021 • edited Loading

the-icarus commented Mar 1, 2021

the-icarus commented Mar 4, 2021

omar-saadoun commented Mar 12, 2021

davidyuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidyuk commented Apr 20, 2021

the-icarus commented Feb 22, 2021 •

edited

Loading