-
Notifications
You must be signed in to change notification settings - Fork 272
Fix prod tsconfig and include ethjs-util types in dist #260
Conversation
@cgewecke thanks for addressing this so quickly, this looks already a lot better with the correct general folder structure and the types from When I pack and test-install this I don't get access to an ES6-style import structure though (or is this technically just not possible?) nor do I get any type information, see the following screenshot: Could you have a look and state what should and should not be working here? 😄 We should also add usage information on the docs here along (on this PR or the subsequent release PR). |
@nebojsa94 @alcuadrado we need to address this here until we can release |
@holgerd77 Very sorry, thanks for double-checking this. I think 173e6bc fixes. Tested locally and TS was able to resolve the types from the dist. Have also removed the added compiler options that were papering over this problem in the tests.
The existing example shows an import statement like this: import { isValidChecksumAddress, unpad, BN } from 'ethereumjs-util' Are you thinking of something more detailed? |
@@ -1,3 +1,4 @@ | |||
/// <reference path="./@types/ethjs-util/index.ts"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just do an import "./@types/ethjs-util/index.ts";
here. I'm not sure if this triple-slash reference would work after compilation, as that index.ts
won't exist in the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is weird but tsc
auto-generates the contents of the @types folder in the dist
creating a .d.ts
file.
dist/
@types/
ethjs-util/
index.d.ts
index.js
index.js.map
And it rewrites the triple slash reference in the dist's main index.d.ts
like this:
/// <reference path="@types/ethjs-util/index.d.ts" />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good now, thanks, tested again by doing a fresh build and local test install, ES6 import and type hints are now working! 👍
@cgewecke the cited example with import { isValidChecksumAddress, unpad, BN } from 'ethereumjs-util'
is actually still not working, but not due the changes here but due to unpad
not available as a function any more, hehe. 😄
But no worry, will give this an update on the release PR.
Fixes two bugs introduced in #248.
include
option in tsconfig.prod.json. Files are no longer written todist/src
anddist/test
is no longer produced.d.ts
file for it and copy everthing into dist appropriately. (More info about this approach on stack overflow here.)