Skip to content

Convert to TS#30

Merged
aptomaKetil merged 2 commits into
masterfrom
feat/types
Mar 9, 2026
Merged

Convert to TS#30
aptomaKetil merged 2 commits into
masterfrom
feat/types

Conversation

@aptomaKetil

@aptomaKetil aptomaKetil commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

No code changes, just adding TS types so I don't have to do declare module '*' in my projects.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds initial TypeScript type declarations for the package and wires typechecking into CI/tests so consumers get typed usage for the logger factory, Logger class, and the Hapi plugin export.

Changes:

  • Added index.ts declaration file describing the public API (hapiLog factory, Logger, and plugin).
  • Added a TypeScript typecheck step (tsc) and a TS “types test” that exercises the declared API.
  • Removed now-redundant JSDoc typedef blocks from runtime JS files and introduced a minimal tsconfig.json.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tsconfig.json Introduces TS configuration for strict, no-emit typechecking of the declarations and type tests.
index.ts Defines the package’s TypeScript declarations for hapiLog, Logger, and plugin.
test/types.test.ts Adds a compile-time-only usage test to validate the new typings.
package.json Exposes the types entrypoint and adds typecheck to the test script; adds TS-related dev deps.
package-lock.json Locks new dev dependencies added for typechecking.
lib/logger.js Removes large JSDoc typedef/comment blocks now covered by the TS declarations.
index.js Removes JSDoc for the exported factory function now covered by the TS declarations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread package.json Outdated
Comment thread index.ts Outdated
Comment thread package.json Outdated
Comment thread package.json Outdated
"version": "11.0.0",
"description": "Hapi Log Plugin",
"main": "index.js",
"types": "index.ts",

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

package.json sets "types": "index.ts". In published packages, the types entry is expected to point to a declaration file (typically index.d.ts); pointing it at a .ts file can cause some consumers/tools to treat it as source and attempt to transpile or include it in builds. Consider renaming this file to index.d.ts (keeping the same export = style) and updating types accordingly.

Suggested change
"types": "index.ts",
"types": "index.d.ts",

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Outdated advice. TS themselves recommend .ts over .d.ts.

@martinj martinj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not rewrite to ts entirely?

Comment thread package.json Outdated
@aptomaKetil aptomaKetil changed the title Add type declarations Convert to TS Mar 9, 2026
@aptomaKetil aptomaKetil requested a review from martinj March 9, 2026 13:46
@aptomaKetil

Copy link
Copy Markdown
Contributor Author

Also removed mocha/nyc/should in favor of native modules, and replaced eslint with biome.

@aptomaKetil

Copy link
Copy Markdown
Contributor Author

I've published this as a beta version, updated pa-services, deployed all four to dev and tested with my integration tests.

@aptomaKetil aptomaKetil merged commit eb9a741 into master Mar 9, 2026
2 checks passed
@aptomaKetil aptomaKetil deleted the feat/types branch March 9, 2026 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants