Modernize the build and convert the client to ES modules - #1220
Open
oskarkrawczyk wants to merge 5 commits into
Open
Modernize the build and convert the client to ES modules#1220oskarkrawczyk wants to merge 5 commits into
oskarkrawczyk wants to merge 5 commits into
Conversation
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
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
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.
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 installno longer resolved on acurrent Node at all.
Toolchain:
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.
PhantomJS functional runner and gives us real two-peer coverage.
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:
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.
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 wouldevaluate ui.js before session.js's body and break its top-level handlers.
Templates and styles:
per-locale templates-.js fetch and the load-order race templates.js
worked around. Untranslated strings are now reported per locale at build
time.
http://localhost:8080placeholder convention became an explicit{{baseUrl}} token.
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:
shortcut could never be removed.
name, which is the settings object inthe one-argument config() form, so trackers only ever fired for the
two-argument form.
later off() call deferring into a dead array.
at load that would have closed storagePrefix before a page could set it.
including togetherjs.js hit youtube.com; it now waits for session start.
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