Fix `types` path and make linter yell #922

webpro · 2023-09-18T17:45:50Z

The "types" field should point to what API Extractor generates in the dist folder.

There is one issue though, added a note.

changeset-bot · 2023-09-18T17:45:53Z

🦋 Changeset detected

Latest commit: c253604

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@kadena-dev/lint-package	Patch
@kadena/chainweb-stream-client	Patch
@kadena/chainweb-node-client	Patch
@kadena/cryptography-utils	Patch
@kadena/pactjs-cli	Patch
@kadena/chainwebjs	Patch
@kadena/cookbook	Patch
@kadena/kda-cli	Patch
@kadena/pactjs	Patch
@kadena/graph	Patch
@kadena/types	Patch
@kadena/tools	Patch
@kadena/client-examples	Patch
@kadena/client	Patch
kadena.js	Patch
@kadena/pactjs-generator	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2023-09-18T17:45:59Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Ignored Deployments

Name	Status	Preview	Updated (UTC)
alpha-docs	⬜️ Ignored (Inspect)	Visit Preview	Sep 26, 2023 10:26am
docs-storybook	⬜️ Ignored (Inspect)	Visit Preview	Sep 26, 2023 10:26am
immutable-records	⬜️ Ignored (Inspect)	Visit Preview	Sep 26, 2023 10:26am
react-ui	⬜️ Ignored (Inspect)	Visit Preview	Sep 26, 2023 10:26am
tools	⬜️ Ignored (Inspect)	Visit Preview	Sep 26, 2023 10:26am

webpro · 2023-09-18T18:01:05Z

packages/libs/pactjs/package.json

@@ -24,7 +24,7 @@
    }
  ],
  "main": "lib/index.js",
-  "types": "lib/index.d.ts",
+  "types": "dist/pactjs.d.ts",


This one causes trouble:

[typescript] Encountered 2 TypeScript issues: [typescript] Error: src/tests/mkCap.test.ts:54:57 - (TS2339) Property 'toPactInteger' does not exist on type 'PactNumber'. [typescript] Error: src/tests/mkCap.test.ts:55:62 - (TS2339) Property 'toPactDecimal' does not exist on type 'PactNumber'. ---- Compile encountered an error (1577ms) ---- -------------------- Finished (1.756s) -------------------- Encountered 2 errors: [typescript] src/tests/mkCap.test.ts:54:57 - (TS2339) Property 'toPactInteger' does not exist on type 'PactNumber'. [typescript] src/tests/mkCap.test.ts:55:62 - (TS2339) Property 'toPactDecimal' does not exist on type 'PactNumber'.

The issue is probably caused by the triple slash directive in PactNumber.ts that ends up in lib/PactNumber.d.ts, but not in dist/pactjs.d.ts.

API Extractor seems to not handle the triple slash directive:

/// <reference path="./extend-bignumber.ts" />

It's just gone (I think it should inline extend-bignumber.ts). To me this seems normal and valid usage of TypeScript. AE seems to support only types and lib references.

I did try a few things with files in tsconfig.json and bundledPackages in api-extractor.json, but to no avail.

Not proud of it, but this does seem to do the trick:

"build": "heft build --clean && cat src/extend-bignumber.ts >> dist/pactjs.d.ts"

There are other ways around this, but making exceptions to the types": "dist/[pkg].d.ts" rule or other TypeScript hacks to me sound even worse.

Any ideas welcome, though!

If we don't come up with a real fix and merge this I could open an issue in the rushstack repo.

If we add the contents of extend-bignumber.ts in PactNumber.ts, would that work?

Unfortunately not, declaration gets stripped, not sure how to work around this.

javadkh2 · 2023-09-20T16:09:02Z

Why? I'd rather use a file generated by typescript itself. it also come with a source map file.

The "types" field should point to what API Extractor generates in the dist folder.

webpro · 2023-09-20T16:23:33Z

Why? I'd rather use a file generated by typescript itself. it also come with a source map file.

The "types" field should point to what API Extractor generates in the dist folder.

This was a request from @alber70g. You can look at https://api-extractor.com for some reasoning under "DTS rollup". It makes sense, until we hit issues like in this PR.

Tbh not a fan myself, just using tsc output is what everyone else does indeed, and would eliminate similar issues.

javadkh2 · 2023-09-20T16:31:00Z

Why? I'd rather use a file generated by typescript itself. it also come with a source map file.

The "types" field should point to what API Extractor generates in the dist folder.

This was a request from @alber70g. You can look at https://api-extractor.com for some reasoning under "DTS rollup". It makes sense, until we hit issues like in this PR.

Tbh not a fan myself, just using tsc output is what everyone else does indeed, and would eliminate similar issues.

we already use all of the good aspects of it. but by using it in the package.json will break source mapping.

alber70g · 2023-09-21T07:39:03Z

packages/tools/cookbook/package.json

@@ -7,6 +7,7 @@
  "license": "ISC",
  "author": "",
  "main": "index.js",
+  "types": "dist/cookbook.d.ts",


I think this one doesn't exist, since there's no heft-build right?

Right, I've also removed this config/api-extractor.json

alber70g · 2023-09-21T07:55:26Z

packages/apps/graph/package.json

@@ -7,6 +7,7 @@
  "license": "ISC",
  "author": "",
  "main": "index.js",
+  "types": "dist/graph.d.ts",


Does it make sense to have this? It's a private package, that's not going to be used in code

Right, I've also removed this config/api-extractor.json

alber70g · 2023-09-21T07:58:45Z

packages/libs/pactjs/package.json

@@ -24,7 +24,7 @@
    }
  ],
  "main": "lib/index.js",
-  "types": "lib/index.d.ts",
+  "types": "dist/pactjs.d.ts",


If we add the contents of extend-bignumber.ts in PactNumber.ts, would that work?

* Fix `types` in every package.json * Make `types` field violation an error * Remove unused .js files * Add docs * Update changelog * Fix up changelog * Not proud * Remove unused api-extractor configs + package.json#types

webpro force-pushed the chore/types-dist-and-lint branch from c7ec451 to 25f6b15 Compare September 18, 2023 17:51

webpro commented Sep 18, 2023

View reviewed changes

sanderlooijenga approved these changes Sep 19, 2023

View reviewed changes

webpro added 4 commits September 20, 2023 12:20

Fix types in every package.json

27fce27

Make types field violation an error

320c4aa

Remove unused .js files

a76b21c

Add docs

1ba8939

webpro force-pushed the chore/types-dist-and-lint branch from 5e23758 to b828c4c Compare September 20, 2023 12:25

webpro added 3 commits September 20, 2023 15:36

Update changelog

6f340f1

Fix up changelog

6b43d20

Not proud

1c8eb92

webpro force-pushed the chore/types-dist-and-lint branch from b828c4c to 1c8eb92 Compare September 20, 2023 13:49

webpro marked this pull request as ready for review September 20, 2023 15:12

webpro requested review from alber70g and Takadenoshi as code owners September 20, 2023 15:12

alber70g approved these changes Sep 21, 2023

View reviewed changes

Remove unused api-extractor configs + package.json#types

c253604

alber70g approved these changes Sep 26, 2023

View reviewed changes

webpro merged commit c8bbec3 into main Sep 26, 2023

webpro deleted the chore/types-dist-and-lint branch September 26, 2023 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `types` path and make linter yell #922

Fix `types` path and make linter yell #922

webpro commented Sep 18, 2023 •

edited

Loading

changeset-bot bot commented Sep 18, 2023 •

edited

Loading

vercel bot commented Sep 18, 2023 •

edited

Loading

webpro Sep 18, 2023

webpro Sep 20, 2023

webpro Sep 20, 2023

alber70g Sep 21, 2023

webpro Sep 26, 2023

javadkh2 commented Sep 20, 2023 •

edited

Loading

webpro commented Sep 20, 2023

javadkh2 commented Sep 20, 2023

alber70g Sep 21, 2023

webpro Sep 26, 2023

alber70g Sep 21, 2023

webpro Sep 26, 2023

alber70g Sep 21, 2023

Fix types path and make linter yell #922

Fix types path and make linter yell #922

Conversation

webpro commented Sep 18, 2023 • edited Loading

changeset-bot bot commented Sep 18, 2023 • edited Loading

🦋 Changeset detected

vercel bot commented Sep 18, 2023 • edited Loading

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

javadkh2 commented Sep 20, 2023 • edited Loading

webpro commented Sep 20, 2023

javadkh2 commented Sep 20, 2023

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

Fix `types` path and make linter yell #922

Fix `types` path and make linter yell #922

webpro commented Sep 18, 2023 •

edited

Loading

changeset-bot bot commented Sep 18, 2023 •

edited

Loading

vercel bot commented Sep 18, 2023 •

edited

Loading

javadkh2 commented Sep 20, 2023 •

edited

Loading