Skip to content

Conversation

SteffenDE
Copy link
Contributor

This is similar to phoenixframework/phoenix_live_view#3789, but it goes one step further and actually changes all files to typescript.

Disclaimer: Claude translated most of the files, I did manual adjustments. This is work in progress.

image image

change documentation to use typedoc
@SteffenDE SteffenDE requested a review from Copilot July 17, 2025 13:36
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR rewrites the JavaScript client into TypeScript, updates build and lint configurations, and adjusts tests accordingly.

  • Converts all assets/js/phoenix sources to TypeScript and updates build output in priv/static
  • Adds TypeScript support in package.json, ESLint, and Typedoc configs
  • Updates test files for semicolons, imports, and TypeScript resolution

Reviewed Changes

Copilot reviewed 41 out of 49 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
typedoc.json Added Typedoc configuration for new TS entry points
priv/static/phoenix.mjs Regenerated client bundle refactored to use const/arrow and TS defaults
package.json Added types, typescript, ts-jest, and new scripts for TS build
mix.exs Updated assets.build alias to include TS compile step
eslint.config.mjs Switched ESLint to use a TypeScript plugin and configs
assets/test/*.js/.ts Updated tests with semicolons, import paths, and TS fixtures
Comments suppressed due to low confidence (4)

package.json:51

  • There’s no build step before running tests, yet tests depend on the compiled TypeScript output. Consider adding a pretest script such as "pretest": "npm run build" to ensure sources are transpiled before Jest runs.
    "test": "jest",

typedoc.json:3

  • [nitpick] Typedoc is pointing at the TS source. If you want to document the distributed API instead, consider pointing entryPoints at the compiled declaration file (e.g., assets/dist/phoenix/index.d.ts) to reflect the actual published API.
  "entryPoints": ["./assets/js/phoenix/index.ts"],

assets/test/socket_test.js:44

  • Tests reference socket.longpollerTimeout, but the client property was renamed to longPollFallbackMs and no default of 20000 is set. Update the test to assert socket.longPollFallbackMs or add a backward‐compatible alias and default value.
      expect(socket.longpollerTimeout).toBe(20000);

eslint.config.mjs:4

  • The ESLint config is importing from typescript-eslint, but the official plugin package is @typescript-eslint/eslint-plugin and you typically also need @typescript-eslint/parser. Update the import and parser settings accordingly.
import tseslint from "typescript-eslint";

this.transport = opts.transport || global.WebSocket || LongPoll;
this.primaryPassedHealthCheck = false;
this.longPollFallbackMs = opts.longPollFallbackMs;
this.longPollFallbackMs = (_a = opts.longPollFallbackMs) !== null && _a !== void 0 ? _a : null;
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

Default longPollFallbackMs is set to null instead of a meaningful default (e.g., 20000ms). Consider using opts.longPollFallbackMs ?? DEFAULT_LONG_POLL_FALLBACK_MS or exposing a longpollerTimeout alias for backward compatibility with existing tests.

Suggested change
this.longPollFallbackMs = (_a = opts.longPollFallbackMs) !== null && _a !== void 0 ? _a : null;
this.longPollFallbackMs = (_a = opts.longPollFallbackMs) ?? DEFAULT_LONG_POLL_FALLBACK_MS;

Copilot uses AI. Check for mistakes.

@SteffenDE SteffenDE mentioned this pull request Sep 1, 2025
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