feat: migrate-lookups-file-flow-to-ts #82
Conversation
Codecov Report
@@ Coverage Diff @@
## master #82 +/- ##
=======================================
Coverage 58.19% 58.19%
=======================================
Files 8 8
Lines 665 665
=======================================
Hits 387 387
Misses 278 278 Continue to review full report at Codecov.
|
…ovskis/subdomain-registrar into feature/migrate-flow-to-ts
src/lookups.ts
Outdated
| } from "blockstack"; | ||
| import { validateStacksAddress } from "@stacks/transactions"; | ||
| import fetch from "node-fetch"; | ||
| //@ts-ignore |
There was a problem hiding this comment.
what was the issue with winston?
There was a problem hiding this comment.
@pradel apparently the lib is not typed and running the suggested "install typed version" command does not fix it, hence the ts-ignore.
There was a problem hiding this comment.
better to declare the module without types in such case, ts-ignore should ideally never be used :)
tsconfig.json
Outdated
| "allowJs": true | ||
| }, | ||
| "include": [ | ||
| "src/lookups.ts" |
There was a problem hiding this comment.
should we include all the src folder here?
There was a problem hiding this comment.
It think no, because in that case all of the files will require being properly typed!
src/lookups.ts
Outdated
| } from "blockstack"; | ||
| import { validateStacksAddress } from "@stacks/transactions"; | ||
| import fetch from "node-fetch"; | ||
| //@ts-ignore |
There was a problem hiding this comment.
better to declare the module without types in such case, ts-ignore should ideally never be used :)
package.json
Outdated
| @@ -48,9 +54,10 @@ | |||
| "start:regtest": "BSK_SUBDOMAIN_CONFIG=config-develop.json BSK_SUBDOMAIN_REGTEST=true npm run start", | |||
| "build": "babel src -d lib", | |||
| "flow": "flow", | |||
| "tsc": "tsc", | |||
There was a problem hiding this comment.
as this is part of the build process, maybe it makes sense to rather edit the build command to be tsc && babel src -d lib?
|
|
Description
Migrated lookups.ts file from Flow to TypeScript as part of a bigger task to migrate the whole project types from Flow to TS.
Following this PR the next task was to start migrating from Flow to Ts:
feat: update eslint config #80
This change is part of M1 of this grant:
Subdomain registrar improvement stacksgov/grants-program#282
After the whole repo is migrated to the TS it should
improve dev experience and productivity as TypeScript is used
across all the other Stacks repos.
Does this introduce a breaking change?
This does not include a breaking change.
Are documentation updates required?
Yes, once this will get a green light, I will add documentation for TS setup either on this branch or the one right after this one.
Testing information
lookups.tsnpm run tscand see no errors