Skip to content

fix(client): ship types for the client exports, and say it is ESM - #2428

Open
alexander-akait wants to merge 2 commits into
mainfrom
feat/client-type-declarations
Open

alexander-akait wants to merge 2 commits into
mainfrom
feat/client-type-declarations

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

Summary

Found by starting the webpack-dev-server side of the client move: none of the five ./client/* exports can be imported by a TypeScript consumer.

tsconfig.client.json only ever checked the client (noEmit: true) and never emitted for it, so no declarations existed and the exports carried no types condition. import WebSocketClient from "webpack-dev-middleware/client/ws" failed outright under noImplicitAny with TS7016.

Declarations alone did not fix it. The client keeps ES module syntax — babel runs with modules: false so webpack can tree-shake it — but the package is CommonJS and nothing says otherwise. Under node16/nodenext resolution both the code and the new declarations therefore resolve as CommonJS, and the default import comes back as the module namespace rather than the class:

Type 'typeof import(".../types/client/clients/WebSocketClient")' provides no match
for the signature 'new (url: string): CommunicationClient'

So this adds both halves:

  1. tsconfig.client.build.json emits declarations for client-src into types/client, wired into build:types.
  2. types conditions on ./client, ./client/sse, ./client/ws, ./client/indicator and ./client/overlay.
  3. A nested package.json marking client and types/client as "type": "module", which is what they have always contained.

No published path changes, which matters: client.webSocketTransport points at client/clients/WebSocketClient.js, so renaming to .mjs was not an option.

Verified from the consumer that reported it, not just locally: webpack-dev-server's lint:types-client fails on that import against published 8.3.0 and passes against a pack of this branch. That is the check this is for, so it seemed worth proving rather than asserting.

What kind of change does this PR introduce?

fix — packaging and types

Did you add tests for your changes?

No new test here; the declarations are build output and what exercises them is a consumer's typecheck, which is where it was verified. If you would like it guarded in CI, the smallest thing that would work is a fixture that imports each ./client/* export under nodenext and typechecks it — say the word and I will add one.

Verified: full build, lint and both typechecks clean, non-browser 6819 passed, end-to-end 110/110. test/logging.test.js fails 74/74 on a clean main too, unrelated to this.

Does this PR introduce a breaking change?

No. Nothing that resolved before stops resolving: the exports keep their existing paths and gain a types condition, and the "type": "module" marker states what those directories already were — a require() of them never worked, since they are ES modules.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

Nothing new — the README already shows these imports in ESM form. This makes that form actually typecheck.

Use of AI

AI-assisted (Claude Code). Used to diagnose why the import failed in a real consumer, work out that declarations alone were not sufficient under nodenext, implement both halves, and verify by packing this branch into webpack-dev-server and re-running its client typecheck.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA


Generated by Claude Code

Summary by CodeRabbit

  • New Features
    • Added TypeScript declarations for the client package and its public exports, including connection clients, overlays, indicators, logging, and update utilities.
    • Added package export mappings so TypeScript can resolve declarations for the client and its subpaths.

Found while starting the webpack-dev-server side of the client move: none of
the five `./client/*` exports could be imported by a TypeScript consumer.

`tsconfig.client.json` only ever checked the client, never emitted for it, so
the exports carried no `types` condition and no declarations existed to point
one at. Importing `webpack-dev-middleware/client/ws` failed outright under
`noImplicitAny` (TS7016).

Declarations alone were not enough. The client keeps ES module syntax —
babel runs with `modules: false` so webpack can tree-shake it — but the
package is CommonJS and nothing said otherwise, so under `node16`/`nodenext`
resolution both the code and the new declarations resolved as CommonJS and
the default import came back as the module namespace, which is not a
constructor. A nested `package.json` in `client` and `types/client` states
what those directories actually contain. No published path changes, which
matters because `client.webSocketTransport` points at one of them.

Verified from the consumer that reported it: webpack-dev-server's
`lint:types-client` failed on the import before, and passes against a pack of
this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
@changeset-bot

changeset-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a552101

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
webpack-dev-middleware Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 0fd90d5c-b85b-40ab-b1f4-e8a47bfbfb37

📥 Commits

Reviewing files that changed from the base of the PR and between a2ff433 and a552101.

📒 Files selected for processing (1)
  • scripts/mark-client-esm.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/mark-client-esm.mjs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.


Walkthrough

The change adds TypeScript declarations for client connection APIs, HMR functions, overlays, indicators, and utility modules. The type build now emits client declarations and marks the client packages as ES modules. Package exports for five client subpaths now provide conditional declaration and runtime JavaScript targets.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to a5521

The client declaration and export changes appear ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a2ff4

The new declarations make existing client exports usable by TypeScript consumers, and the ES-module markers change how those files are resolved. No new attacker-controlled runtime path was demonstrated. Build ordering and incomplete downstream coverage leave some packaging uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The identified change affects consumers of the five published client subpaths and their type and module resolution. Their JavaScript export targets remain unchanged; broader downstream exposure has not been established.

Security Findings and Attack Paths

  • inferred — No new attacker-controlled path to the inspected message or WebSocket handlers was demonstrated: the newly declared entrypoint functions and handlers exist in the runtime, and the export change retains their JavaScript targets. This is not a claim of complete security coverage.

Trust Boundaries and Controls

  • observed — The browser client parses received message data before dispatch and catches parsing or processing errors at that listener. Type declarations do not change those runtime controls.

Resilience and Maintainability Implications

  • inferred — Successful normal publication is gated by a completed build, which limits exposure to a failed marker write. The parallel build and sequential marker writes do not, however, guarantee an all-or-nothing intermediate workspace state.

Hardening Proposals

  • proposed — Sequence marker creation after client output is complete and validate both module markers in the package artifact, reducing reliance on timing or an external packager observing only completed workspace state.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: adding types for client exports and marking the client as ESM.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3c2d5b6d-8397-4b04-928c-5c5bdfc5926b

📥 Commits

Reviewing files that changed from the base of the PR and between 4083960 and a2ff433.

📒 Files selected for processing (18)
  • .changeset/client-type-declarations.md
  • package.json
  • scripts/mark-client-esm.mjs
  • tsconfig.client.build.json
  • types/client/clients/EventSourceClient.d.ts
  • types/client/clients/WebSocketClient.d.ts
  • types/client/clients/createSocket.d.ts
  • types/client/index.d.ts
  • types/client/indicator.d.ts
  • types/client/overlay.d.ts
  • types/client/package.json
  • types/client/process-update.d.ts
  • types/client/theme.d.ts
  • types/client/utils/get-hot.d.ts
  • types/client/utils/log.d.ts
  • types/client/utils/reload.d.ts
  • types/client/utils/send-message.d.ts
  • types/client/utils/strip-ansi.d.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Comment thread package.json
Review finding, and reproducible: `build` runs every `build:*` in parallel,
so `build:types` can reach the marker script before babel has created
`client`, and the write fails with `ENOENT`. It went unnoticed because
`tsc` is the slower of the two here, so babel always happened to win — and
CI happened to agree.

`build:types` on its own after a `clean` fails every time rather than
occasionally, which is what made it easy to confirm and now easy to check:
that sequence errored before this and exits 0 after it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
@codecov

codecov Bot commented Sep 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.91%. Comparing base (4083960) to head (a552101).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2428   +/-   ##
=======================================
  Coverage   95.91%   95.91%           
=======================================
  Files          18       18           
  Lines        2006     2006           
=======================================
  Hits         1924     1924           
  Misses         82       82           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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