Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript declarations #188

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Typescript declarations #188

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
98dd2b8
added automatic typescript declarations
Aug 15, 2024
251d772
bugfix tsconfig path
Aug 15, 2024
74123b3
move build-types from prepare to prepack
Aug 15, 2024
8196c9b
added types for testing
Aug 15, 2024
0c15cc8
change exports to main
Aug 15, 2024
022e817
add rimraf and remove temporary types
Aug 15, 2024
33fc7ac
add types back to gitignore
Aug 15, 2024
9e55f64
Merge branch 'main' into typescript-declarations
Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ npm-debug.log
package-lock.json
.nyc_output
dist
/types/
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# @digitalbazaar/vc ChangeLog

### Added
- Generate TypeScript declarations for published packages.

## 7.0.0 - 2024-08-01

### Added
Expand Down
16 changes: 12 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@
},
"license": "BSD-3-Clause",
"type": "module",
"exports": "./lib/index.js",
"main": "./lib/index.js",
"types": "./types/index.d.ts",
"files": [
"lib/**/*.js"
"lib/**/*.js",
"types/**/*.d.ts",
"types/**/*.d.ts.map"
],
"dependencies": {
"@digitalbazaar/credentials-context": "^3.1.0",
Expand All @@ -45,6 +48,7 @@
"@digitalbazaar/ed25519-verification-key-2018": "^4.0.0",
"@digitalbazaar/multikey-context": "^2.0.1",
"@digitalbazaar/odrl-context": "^1.0.0",
"@types/node": "^22.3.0",
"c8": "^10.1.2",
"chai": "^4.5.0",
"cross-env": "^7.0.3",
Expand All @@ -64,6 +68,8 @@
"klona": "^2.0.6",
"mocha": "^10.7.0",
"mocha-lcov-reporter": "^1.3.0",
"rimraf": "^6.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is rimraf really a main dependency? https://www.npmjs.com/package/rimraf

This seems like an entry in devDepencies also do @types/node and typescript have to be in the dependencies for all users? It seems like that introduces a lot of bloat in this library. Are you sure those aren't devDependencies also?

Copy link
Author

@spetrac spetrac Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those definitively belong in the devDependencies and they are already.

You are also right that rimraf is not a necessary dependency and only used during development to clear up between builds. You could also use the usual rm command on linux, but then it would not be cross-plattform compatible. You can also discard the clear-up completely, because the build step is meant to be used automatically when publishing a package, but then it could be irritating during development.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The rimraf dependency is only for the convenience of the developer, so that previous builds must not be cleared manually during development. You can see it's usage in the package.json-build-types-script.
  • The typescript dependency is obviously necessary to build the types. You can see usage of tsc in the build-types-script.
  • The @types/node dependency is used to give typescript the necessary types in the NodeJS environment, so that it will not warn of missing types and validate the usage of internal classes and API.

These are just the basics of typescript development and should not really be a surprise. If you install the package in production those dependencies will not even be downloaded. The published types should then already be included in the package. Alternatively you can publish proper types in the DefinitelyTyped repository, but then you should not just use automatically generated types.

"typescript": "^5.5.4",
"uuid": "^10.0.0",
"veres-one-context": "^12.0.0",
"webpack": "^5.93.0"
Expand Down Expand Up @@ -94,6 +100,8 @@
"lint": "eslint 'lib/**/*.js' 'test/**/*.js'",
"coverage": "cross-env NODE_ENV=test c8 npm run test-node",
"coverage-ci": "cross-env NODE_ENV=test c8 --reporter=lcovonly --reporter=text-summary --reporter=text npm run test-node",
"coverage-report": "c8 report"
"coverage-report": "c8 report",
"prepack": "npm run build-types",
"build-types": "rimraf types & tsc -p tsconfig.types.json"
}
}
}
12 changes: 12 additions & 0 deletions tsconfig.types.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"allowJs": true,
"declaration": true,
"emitDeclarationOnly": true,
"outDir": "types",
"declarationMap": true
},
"include": [
"lib/**/*.js"
]
}