Skip to content

Modernize the build and convert the client to ES modules - #1220

Open
oskarkrawczyk wants to merge 5 commits into
developfrom
claude/client-modernize-voice-video-hicm4p
Open

Modernize the build and convert the client to ES modules#1220
oskarkrawczyk wants to merge 5 commits into
developfrom
claude/client-modernize-voice-video-hicm4p

Conversation

@oskarkrawczyk

Copy link
Copy Markdown
Member

The client was ~10k lines of ES5 AMD modules loaded by a vendored RequireJS
2.1.8, built by Grunt 0.4 against node ~0.12.7, styled with LESS 1.3, and
tested by doctest.js under PhantomJS. npm install no longer resolved on a
current Node at all.

Toolchain:

  • esbuild replaces Grunt's copylib/maybeless/substitute/config-requirejs
    chain. One build/build.mjs produces dist/togetherjs.js (IIFE, still a
    drop-in <script src>), dist/togetherjs.min.js (actually minified now —
    the old requirejs task ran with optimize:"none", so togetherjs-min.js
    never was), dist/togetherjs.esm.js for npm, plus separate recorder and
    walkabout entries.
  • vitest replaces the doctest.js unit tests; Playwright replaces the
    PhantomJS functional runner and gives us real two-peer coverage.
  • ESLint flat config + Prettier replace jshint/csslint.

Sources moved to src/ and converted from define([...]) to import/export.
esbuild cannot consume AMD (it leaves define() as a free call and drops the
dependencies), so the toolchain and the module conversion had to land
together rather than as separate steps.

Two structural changes the single-bundle format forced:

  • The entry split into src/core/togetherjs.js (the TogetherJS object,
    config, events) and src/index.js (boot). Under RequireJS the load order
    made the window.TogetherJS global safe to read at module scope; in a
    bundle it is not, so every module now imports TogetherJS explicitly.
  • Config reads that used to happen at module scope now happen lazily.
    The bundle evaluates when the <script> runs, which is before the host
    page's configuration has been applied — storagePrefix, includeHashInUrl,
    ignoreMessages and the template language were all being read too early.

Import cycles (session<->ui, peers<->ui) are handled by a small registry in
src/core/registry.js rather than static imports, preserving the deferred
require("ui") semantics the AMD code relied on; a static import would
evaluate ui.js before session.js's body and break its top-level handlers.

Templates and styles:

  • All five locales are compiled into the bundle at build time, removing the
    per-locale templates-.js fetch and the load-order race templates.js
    worked around. Untranslated strings are now reported per locale at build
    time.
  • The http://localhost:8080 placeholder convention became an explicit
    {{baseUrl}} token.
  • LESS is retired. The sources used only variables, four vendor-prefix
    mixins and nesting, but also rule-as-mixin calls that made a hand
    conversion risky, so they were compiled once and committed as plain CSS,
    with obsolete vendor prefixes stripped and the design tokens hoisted into
    :root custom properties.

Fixes found along the way:

  • TogetherJS.removeShortcut() called addEventListener, so the alt-T
    shortcut could never be removed.
  • _configTrackers was indexed by name, which is the settings object in
    the one-argument config() form, so trackers only ever fired for the
    two-argument form.
  • emit() returned early without clearing _listenerOffs, leaving every
    later off() call deferring into a dead array.
  • initIdentityId.done was assigned but never read, and fired a storage read
    at load that would have closed storagePrefix before a page could set it.
  • youtube.js fetched the YouTube iframe API at module scope, so merely
    including togetherjs.js hit youtube.com; it now waits for session start.
  • Errors thrown in the start chain were swallowed by jQuery Deferreds,
    leaving TogetherJS silently half-started; they are surfaced now.

Deleted: Gruntfile, devserver, Travis config, the Firefox addon (built with
cfx, removed from Firefox in 2017), the legacy hub/ server superseded by
hub-worker/, analytics.js (classic ga.js), the vendored require/almond/
jquery/tinycolor copies, and the doctest.js framework.

jQuery is still present, from npm rather than vendored; removing it is the
next step.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01HgRCPNvLVGLYVox8Va8uJh

claude added 5 commits August 11, 2026 21:43
The client was ~10k lines of ES5 AMD modules loaded by a vendored RequireJS
2.1.8, built by Grunt 0.4 against node ~0.12.7, styled with LESS 1.3, and
tested by doctest.js under PhantomJS. `npm install` no longer resolved on a
current Node at all.

Toolchain:

- esbuild replaces Grunt's copylib/maybeless/substitute/config-requirejs
  chain. One build/build.mjs produces dist/togetherjs.js (IIFE, still a
  drop-in <script src>), dist/togetherjs.min.js (actually minified now —
  the old requirejs task ran with optimize:"none", so togetherjs-min.js
  never was), dist/togetherjs.esm.js for npm, plus separate recorder and
  walkabout entries.
- vitest replaces the doctest.js unit tests; Playwright replaces the
  PhantomJS functional runner and gives us real two-peer coverage.
- ESLint flat config + Prettier replace jshint/csslint.

Sources moved to src/ and converted from define([...]) to import/export.
esbuild cannot consume AMD (it leaves define() as a free call and drops the
dependencies), so the toolchain and the module conversion had to land
together rather than as separate steps.

Two structural changes the single-bundle format forced:

- The entry split into src/core/togetherjs.js (the TogetherJS object,
  config, events) and src/index.js (boot). Under RequireJS the load order
  made the window.TogetherJS global safe to read at module scope; in a
  bundle it is not, so every module now imports TogetherJS explicitly.
- Config reads that used to happen at module scope now happen lazily.
  The bundle evaluates when the <script> runs, which is before the host
  page's configuration has been applied — storagePrefix, includeHashInUrl,
  ignoreMessages and the template language were all being read too early.

Import cycles (session<->ui, peers<->ui) are handled by a small registry in
src/core/registry.js rather than static imports, preserving the deferred
`require("ui")` semantics the AMD code relied on; a static import would
evaluate ui.js before session.js's body and break its top-level handlers.

Templates and styles:

- All five locales are compiled into the bundle at build time, removing the
  per-locale templates-<lang>.js fetch and the load-order race templates.js
  worked around. Untranslated strings are now reported per locale at build
  time.
- The `http://localhost:8080` placeholder convention became an explicit
  {{baseUrl}} token.
- LESS is retired. The sources used only variables, four vendor-prefix
  mixins and nesting, but also rule-as-mixin calls that made a hand
  conversion risky, so they were compiled once and committed as plain CSS,
  with obsolete vendor prefixes stripped and the design tokens hoisted into
  :root custom properties.

Fixes found along the way:

- TogetherJS.removeShortcut() called addEventListener, so the alt-T
  shortcut could never be removed.
- _configTrackers was indexed by `name`, which is the settings *object* in
  the one-argument config() form, so trackers only ever fired for the
  two-argument form.
- emit() returned early without clearing _listenerOffs, leaving every
  later off() call deferring into a dead array.
- initIdentityId.done was assigned but never read, and fired a storage read
  at load that would have closed storagePrefix before a page could set it.
- youtube.js fetched the YouTube iframe API at module scope, so merely
  including togetherjs.js hit youtube.com; it now waits for session start.
- Errors thrown in the start chain were swallowed by jQuery Deferreds,
  leaving TogetherJS silently half-started; they are surfaced now.

Deleted: Gruntfile, devserver, Travis config, the Firefox addon (built with
cfx, removed from Firefox in 2017), the legacy hub/ server superseded by
hub-worker/, analytics.js (classic ga.js), the vendored require/almond/
jquery/tinycolor copies, and the doctest.js framework.

jQuery is still present, from npm rather than vendored; removing it is the
next step.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgRCPNvLVGLYVox8Va8uJh
jQuery was the client's last vendored dependency and its promise primitive.
Both are gone.

src/dom/dom.js is a ~700-line stand-in providing the subset of the jQuery API
the client actually called — nothing more. Rewriting ~500 call sites as
querySelectorAll + loops would have ballooned ui.js and forms.js and invited
transcription bugs, so the call sites are almost unchanged; only the import
moved. It is covered by 43 unit tests pinning the behaviours those call sites
depend on, including the surprising ones: attr() yielding undefined rather
than null for a missing attribute, `return false` in a handler meaning
preventDefault + stopPropagation, and .end() restoring the previous set.

Two jQuery behaviours needed explicit support rather than deletion:

- The non-standard pseudo-selectors. `:visible` is used by windowing.js and
  `:password` is the default value of the ignoreForms config, and neither is
  valid CSS, so they are translated before hitting querySelectorAll.
- Progress notifications on deferreds. who.js reports users as it discovers
  them and ui.js refreshes the invite list from each notification, so
  util.Deferred keeps notify()/progress() on top of a native promise. Unlike
  a jQuery Deferred it reports unhandled rejections instead of swallowing
  them — which is how the silent start-up failure in the previous commit
  managed to hide.

jqueryPlugins.js is replaced by src/dom/animate.js. Every animation there ran
through jQuery `step:` callbacks tweening a fake `borderSpacing` property and
writing four vendor-prefixed transforms per frame; they are Web Animations
and CSS transitions now, and the typing indicator is a CSS keyframe animation
that respects prefers-reduced-motion instead of a setInterval writing opacity.

Dead code removed, all of it dead because jQuery 1.9 removed the API it
depended on and jqueryPlugins.js only faked back $.browser.mobile:

- startup.js branched on $.browser.msie, undefined since 1.9.
- ui.js bound the dock anchor with .toggle(fn1, fn2), a signature removed in
  1.9, so collapsing the dock had silently done nothing for years. Restored
  as an explicit toggle.
- cursor.js's rotateCursorDown had no callers and its own comment said it no
  longer did anything.
- jqueryPlugins.js tested `matchMedia("screen and (max-screen-width: 480px)")`
  — not a valid media feature, so it never matched. $.isMobile() uses
  `(pointer: coarse)`.

Also: $.ajax in playback.js and $.each in forms.js became fetch and a normal
loop; $(window).unload in recorder.js became pagehide.

e2e coverage grew to cover what this change touched most: cursor propagation
between peers, and opening the chat and share windows (which exercises the
replaced animation code). The fixture now presents as a returning user,
because the first-run walkthrough is a modal whose backdrop covers the dock.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgRCPNvLVGLYVox8Va8uJh
The old webrtc.js could not work with more than two people, and was fragile
with two. It kept a single module-level RTCPeerConnection for the entire room
and broadcast its offers to everyone, so a third participant made every client
fight over the same connection. ICE candidates went into a scalar that each
new candidate overwrote, and any arriving before setRemoteDescription were
dropped. abort() discarded the connection without close(). toggleMute() was an
empty stub, so the muted state was unreachable and there was no way to hang up.

src/rtc/ replaces it:

  media.js       microphone and camera, acquired independently
  connection.js  one peer connection, using perfect negotiation
  mesh.js        the set of connections, and the signaling
  ui.js          dock buttons and video tiles

Signaling is addressed. rtc-description and rtc-ice carry a `to`, which
session.js filters on; rtc-offer/rtc-answer/rtc-abort are gone, since perfect
negotiation needs one description message and no abort channel. The filtering
is client-side: hub-worker's room.ts is deliberately a blind relay that
inspects nothing, and teaching it message types would couple client and server
versions to save bandwidth on a mesh that is capped at six peers anyway. This
is addressing, not privacy — every client still receives every frame, exactly
as before.

Getting three peers to connect reliably took four fixes, each a real defect:

- Both sides opened a connection to each other, so the two objects could end
  up negotiating against each other's discarded counterpart. Perfect
  negotiation resolves colliding offers on one connection; it cannot merge
  two. The peer with the lower clientId now opens, and the other creates its
  connection when the offer arrives.
- The answering side also added transceivers, which fires negotiationneeded,
  so it offered back at the moment it was about to answer — turning every
  single connection setup into a collision. It now stays quiet until it has
  applied the first remote description.
- connectTo() awaits the ICE configuration, so two callers racing for the
  same peer built two connections and one was orphaned in the map, never
  negotiating. Attempts are now shared and carry an epoch, so a creation that
  was in flight when the peer was reset discards itself.
- Candidates arriving while a connection was still being created were
  dropped. They are buffered per peer.

A peer that offers before the other side has joined the call gets no answer,
because a client that has not joined cannot respond. mesh.start() therefore
announces with rtc-join, and a peer parked in have-local-offer with no remote
description rebuilds that connection. Only that case: resetting anything not
yet "connected" also kills connections that are merely still gathering, and
with three peers announcing joins nothing ever settles.

Behaviour that did not exist before: mute (track.enabled, no renegotiation,
broadcast as rtc-state so peers can show a badge — a disabled audio track
still sends silence, so there is no other way to tell), camera on/off mid-call
via replaceTrack, hang-up, ICE restart on failure, device pickers, and video
tiles. Turning the camera off stops the track rather than disabling it, so the
hardware capture light goes out.

Configuration gains enableVideo (off by default), iceServers, getIceServers
for short-lived TURN credentials, and maxRtcPeers. Without TURN, symmetric-NAT
peers still fail — but visibly, and the docs will say so rather than repeating
the 2013 apology.

session.RTCSupported now also requires a secure context: navigator.mediaDevices
is undefined on http:// pages, where the old check passed and then threw inside
getUserMedia. That case gets its own dialog, since no permission prompt fixes it.

Also removed: the dead camera-avatar block bound to nine DOM ids that do not
exist in interface.html, and the mobile-only handler in ui.js that bound a
second click listener always claiming RTC was unsupported.

Tested with seven end-to-end cases covering two- and three-peer calls,
simultaneous joins (the glare the old code could not survive), enabling video
mid-call without dropping audio, mute propagation, a peer leaving, and hang-up.
They assert on getStats() packet counts, not just connectionState: ICE
completing does not prove media is flowing. Chromium's fake media devices
produce nothing in this container, so getUserMedia is backed by canvas and
WebAudio tracks — real MediaStreamTracks that negotiate and flow normally.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgRCPNvLVGLYVox8Va8uJh
The WebRTC documentation was thirteen years out of date: it told readers they
needed Firefox Nightly, that support was coming "sometime in 2013", and
apologised for a missing TURN server by linking a 2013 issue. It now describes
what the client actually does — joining and muting, the camera button, why the
page has to be served over https, how to supply your own TURN servers (both
static and minted-per-session), and why calls are capped at six people.

The new configuration keys are documented: enableVideo, iceServers,
getIceServers and maxRtcPeers.

README build instructions replace the Grunt ones, and src/README.md replaces
the old per-module list. It leads with the two conventions that are not
obvious from reading any single file: core/togetherjs.js is imported rather
than read off window, and configuration must be read lazily because the bundle
evaluates before the host page has configured anything.

Deleted module-descriptions.json, which nothing had read since the docco task
went away.

The Playwright config now finds a browser in both places: it uses the Chromium
this dev container pins when that path exists, and otherwise lets Playwright
resolve the one `playwright install` fetched — so the suite runs in CI without
further configuration.

Note: a GitHub Actions workflow to run lint/build/unit/e2e on Node 22 is *not*
included, because this app lacks permission to push .github/workflows/. Travis
is gone, so there is currently no CI; adding a workflow is a one-file follow-up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgRCPNvLVGLYVox8Va8uJh
Every locale carries its own copy of interface.html, so anything left in the
markup is paid for five times. The comments are notes to whoever edits the
file and mean nothing at runtime.

Cuts the minified bundle from 508KB to 321KB, and 83KB to 65KB gzipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgRCPNvLVGLYVox8Va8uJh
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.

2 participants