Skip to content

Comments

Chore: modernize and upgrade system#22

Draft
herostrat wants to merge 4 commits intoSignalK:masterfrom
herostrat:upgrade_ts
Draft

Chore: modernize and upgrade system#22
herostrat wants to merge 4 commits intoSignalK:masterfrom
herostrat:upgrade_ts

Conversation

@herostrat
Copy link

TBH this does a lot of things at once, but I already did the conversion and better als Draft than forgotten.

So this basically converts the code in ts, fixes problems this conversion found, adds tests for critical codepath (in my mind) and also implements basic CI to see when something regresses.

Leaving it as draft until the other stuff landed

Fixes needed for ts conversion:

  1. Unsafe access to options
  • Symptom: useNetworkTime accessed options.preferNetworkTime without guarding options, which can throw when start() is called with no options.
  • Impact: Plugin could crash on startup or on first navigation.datetime message when options are omitted.
  • Root cause: Missing undefined checks in useNetworkTime and call sites.
  • Fix: Use optional chaining (options?.preferNetworkTime) and default behavior when options are not provided.
  1. Undeclared variable and incorrect command output handling
  • Symptom: validSources was not declared and the result from execSync was used directly in a numeric comparison.
  • Impact: Implicit global variable + unreliable comparison, causing network time detection to behave incorrectly or inconsistently.
  • Root cause: Missing const and not converting the command output to a number.
  • Fix: Declare const validSources = parseInt(output.toString(), 10) and handle NaN safely.

These fixes were applied in the TypeScript version to avoid runtime errors and make the behavior deterministic.

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.

1 participant