-
-
Notifications
You must be signed in to change notification settings - Fork 184
refactor: convert requestResponse to TypeScript #2284
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.
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
Open
dirkwa
wants to merge
5
commits into
SignalK:master
Choose a base branch
from
dirkwa:feature/ts-convert-request-response
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Convert requestResponse.js to TypeScript with strict type definitions. Remove lodash dependency by using native Object.values/Object.keys.
Convert put.js to TypeScript with proper type definitions, removing the need for eslint-disable workarounds. Define interfaces for app parameters (PutApp, PathApp, NotificationApp) that specify only the properties each function actually uses. Use branded types (Context, Path, SourceRef) from @signalk/server-api for type-safe delta construction, following existing codebase patterns. Add missing 'self' property to FullSignalK type declaration to match runtime behavior.
Convert the WebSocket interface from JavaScript to TypeScript as part of the ongoing migration effort. Changes: - Rename src/interfaces/ws.js to ws.ts with full type annotations - Add local Primus type declarations (src/@types/primus.d.ts) since the official @types/primus is incompatible with SignalK's runtime mutation pattern - Add AccessRequestData interface to requestResponse.ts for proper typing of access request replies - Add @types/cookie and @types/jsonwebtoken dev dependencies The conversion maintains all existing functionality while adding type safety for WebSocket connections, delta handling, backpressure management, and authentication flows.
Contributor
Author
|
The reason why I work in this refactor with a incremental PR approach are the dependencies: |
The interfaces/index.js dynamically loads interface modules using require(), which expects module.exports to be the function directly. ES module default exports compile to exports.default, causing "theInterface is not a function" error at runtime. Change from ES module default export to named function with explicit module.exports assignment to maintain compatibility with the existing CommonJS interface loading pattern.
Convert tokensecurity.js to tokensecurity.ts with full type safety, completing the chain of 4 interdependent JS→TS conversions. Add typed interfaces for internal structures (TokenSecurityApp, JWTPayload, CookieOptions, SecurityOptions, AccessRequestRecord). Export shared types from security.ts (SkPrincipal, Delta, etc.). Update supporting files with proper SkPrincipal type usage.
Contributor
Author
|
@tkurki |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Convert the interdependent request/response handling chain from JavaScript to TypeScript with proper type safety.
Files converted
requestResponse.js→requestResponse.tsput.js→put.tsinterfaces/ws.js→interfaces/ws.tstokensecurity.js→tokensecurity.tsSupporting changes
security.ts(SkPrincipal, Delta, SecurityStrategy interface)src/@types/primus.d.ts)dummysecurity.tsto use proper types instead of eslint-disableApproach
These four files form a dependency chain and needed to be converted together. Each file now has proper TypeScript types without using
any,eslint-disable, or underscore-prefixed unused parameters. Intentionally unused parameters usevoidexpressions for clarity.