Custom Dashboard SDK: promise client and React bindings for the postMessage bridge - #12
Open
gustavo-aguiar wants to merge 12 commits into
Open
Custom Dashboard SDK: promise client and React bindings for the postMessage bridge#12gustavo-aguiar wants to merge 12 commits into
gustavo-aguiar wants to merge 12 commits into
Conversation
The host moved to a request/response postMessage bridge, so a shell now has to generate correlation ids, match responses, and survive the several situations where the host drops a reply without answering. That plumbing belongs here rather than in every uploaded HTML file.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…frame src The typed path flowed straight into iframe.src, which accepts javascript:. CodeQL flagged it as js/xss-through-dom. Examples get copied, and the SDK README tells authors not to do exactly this.
tago-io/admin#5209 gives the host its own `timeout` code for a query that ran past its execution deadline. The SDK was already using that name for a reply that never arrived, and the two need different remedies: narrow the query versus the dashboard went away. Also picks up the host's new plan_limit and rate_limited codes.
tago-io/admin#5209 split timeouts, rate limits and plan caps out of that code, so the comments describing it as a catch-all no longer hold.
… codes Three findings from reviewing this branch: pending-pool kept an LRU of timed-out ids that nothing read, so the promised diagnostic did not exist. settleResponse now consults it and logs one console.debug naming the request, which separates a late reply from a query that quietly did nothing. An id the client never issued stays silent. style.get() handed every client the same module-level object, so a mutation in one shell reached all of them. Now one per instance. plan_limit, timeout and rate_limited were added to the host union without a test. They are in the error-code table now, plus a case proving an unrecognized code is forwarded rather than flattened.
Contributor
Author
|
Block Description alignmentStale
Thread consolidationPrior leads
In this PRTwo of the three Blockers are docs-and-plumbing around a client that itself holds up well. The third is a fix from the last push that stopped one line short. Blockers
Improvements
Praise
Risk (CIA)Likelihood: 🟢 Low | Impact: 🟢 Low | Exposure: 🟢 Low. Outside this PRPre-existing
Suggestions
|
…error table Three findings from re-reviewing this branch, plus two the pass turned up. The error table still described `timeout` as "The host never answered", which is what `no_response` means since 854ba72. The host owns `timeout` for a query past its execution deadline, so an author following the table branched on the opposite failure and got the exact collision the rename removed. The table now splits host codes from SDK codes and carries all thirteen, including plan_limit, rate_limited, no_response and internal. setStyle adopted the inbound payload. One MessageEvent reaches every listener carrying the same data object, so after a single style message two clients in a document shared what style.get() hands out, and a mutation through one showed up in the other. The per-instance {} from f971e5d only covered the state before the first message, and the test asserted just that half. Copy on assignment, and the test now dispatches a message first. The in-flight release deleted its coalescing key unconditionally, so a settle from a stopped run dropped whatever held that key by then. stop(), start() and an identical call land in one tick under StrictMode, which is the case coalescing exists for, and there it cost a second execution. Retract only our own entry. Two more from the same pass. An unrecognized host code is forwarded verbatim, so the runtime value sits outside the closed error union and a consumer's exhaustive switch could claim a never default it does not have; TDashboardErrorCode now has an open tail, with TKnownErrorCode kept closed for anyone who wants that exhaustiveness. And normalizeSqlRunResult dropped every non-string column despite a comment promising a cosmetic columns change would not take down a shell, which would have meant an empty header row above populated rows; it reads .name off a record now. Reverting any of the three code fixes kills exactly one test and no others.
The Pages job copied the dashboard examples into _site/examples/dashboard/ while the bundle went to _site/dist/, so each example's relative ../dist/ path resolved to _site/examples/dist/, which nothing populates. Every published example loaded no SDK, and the || true guards kept the job green while it happened. They go flat alongside the widget examples, where ../dist/ resolves the same way it does in the package. No filename collides. Nothing was typechecking tests/. `vp check` covers lint and format, and tsup typechecks src/ on its way to the .d.ts, so a type-level assertion in a test could not fail the build. A repo-wide check:types step fixes that and passes clean on all five packages today. The pull_request trigger named master, which does not exist in this repo, so the PR check never fired and only the push trigger was reporting.
Running the browser tooling against this repo drops console logs, page snapshots and screenshots into .playwright-mcp/. None of it is source, and leaving it untracked invites committing a session log by accident.
…he narrow code type Findings from an adversarial pass over the previous two commits, which refuted five of the six fixes they made. useSqlQuery retained data across a change of query id, not just across a refetch of the same query. Switching query and failing left the previous query's rows on screen under the new query's identity, which the docblock never claimed and which a shell has no way to detect. It now clears on an identity change, and the two halves are pinned separately: cleared on switch, still kept across a refetch. TKnownErrorCode was never exported from the package entry, so the README instruction to narrow with it for an exhaustive switch could not be followed. The type test now imports from the entry rather than the deep src/types path, which is why it did not catch this; un-exporting the type is a typecheck failure today. The error table's new Raised by column was wrong on two of thirteen rows. The SDK raises api_error in four places for a malformed reply, and bad_request locally for a query id that fails its format check, which is in fact the only way a shell normally sees that code. Both are marked Both, and the claim that bad_params covers only 400s is corrected: the host folds every unmapped 4xx, 422 included. The coalescing regression test did not pin its fix. In isolation it passed against an implementation with no cleanup at all, because "no third request" is equally true of a key that leaked. It now settles the survivor and demands a fresh request. A second test covers releasing a settled key while an unrelated request is in flight, which nothing covered and which a plausible wrong guard passes. The Pages job copied the examples unconditionally while guarding the bundle, so a missing bundle still published pages whose relative script src 404s, green. The example copy now rides on the bundle having landed, and warns when it has not. check:types reached two of five packages: react and dashboard-react excluded tests/ from tsconfig. rootDir went with it, which only constrains emit and tsc here only ever runs --noEmit, exactly as core and dashboard already do. js keeps both, since its one test is co-located in src and was covered all along. Also drops a dead README link to an examples directory that was never committed.
recordsEqual compared four of the twelve fields TDataRecord declares, so any edit landing only in the other eight was judged identical and the incoming record was thrown away. metadata is the common way in: editDeviceData writing metadata leaves value and time untouched, so a recoloured card kept its old colour, a swapped file never loaded its new URL, and a tracker reporting a new position with the same reading left its map pin frozen. Reloading showed the edit, because an empty store adopts the first payload verbatim, which is why it reads as broken until refresh. The comparison now covers every declared field, including the deprecated origin and bucket, since a field left out is a field whose edits a widget never sees. metadata and location are compared by value through one small recursive helper. Reference and shallow both fail here and not as a matter of taste: realtime payloads cross postMessage, which structured-clones them, so metadata is a fresh object on every tick and coordinates is a fresh array. JSON.stringify is out because it is key-order sensitive, and a reordered metadata object describes the same record. An absent key and a key holding undefined count as equal, because the platform sends JSON and the parent's own edit path emits old_value: undefined. Structural sharing survives, which matters as much as the fix: an unchanged tick still returns the same array and the same record objects, so memoized rows do not repaint. Cost measured on 1000-record ticks: 0.26ms to 0.46ms for typical records, and 0.34ms to 4.7ms for records carrying fat metadata. Verified against a real custom widget on a real dashboard, bound to a real device, editing through the widget's own editData so the write, the socket echo and the merge are all the production path. Same widget A/B/A across the two bundles: with the fix onRealtime fires again and the cell repaints, with the shipped bundle the API confirms "Device Data Updated" and onRealtime never fires at all. The vanilla SDK was the worst affected, since its callback is gated on array identity. Tests cover the reported metadata case, nested metadata, metadata arrays, location, one generated case per declared field from a Required<TDataRecord> fixture so the coverage grows with the type, and the three no-op pins: an identical tick, a key-order-only difference and an explicitly undefined key.
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
2 participants
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.
Summary
Adds
@tago-io/custom-dashboardand@tago-io/custom-dashboard-react: a promise client and React bindings for the Custom Dashboard postMessage bridge. Both packages shipprivate: trueand no changeset, so nothing publishes until a deliberate release PR.It also carries an unrelated fix to
@tago-io/custom-widget-core, reported separately: the defaultmergerealtime strategy compared four of the twelve fields a record declares, so an edit landing only inmetadata,location,unit,group,deviceorcreated_atwas judged identical and discarded. That is a widget SDK bug, not a Dashboard SDK one, and it is here because it was found while this branch was open. Say the word and it moves to its own PR offmain.Two repo-wide CI changes come along, both small and both load-bearing here.
ci.ymlgains apnpm -r check:typesstep, becausevp checkcovers lint and format only and tsup typecheckssrc/on its way to the.d.ts, so nothing was typecheckingtests/and a type-level assertion in a test could not fail the build. Itspull_requesttrigger also moves from the non-existentmastertomain, which is what makes the PR check fire at all.Why
Custom Dashboard has no SDK. The entire public contract is a snippet hardcoded in Admin's empty state, so every author hand-rolls the same plumbing. Since the host moved to a request/response bridge in tago-io/admin#5185, that plumbing grew teeth: the shell has to generate correlation ids, match responses to requests, and handle nine error codes. And the host drops replies without answering in several situations, so a naive promise wrapper hangs forever.
The SDK owns exactly that, and nothing else. It does not reshape the protocol:
sql.runtakesparamsin the wire shape, and the SDK reports the theme rather than writing to the author's DOM.Test plan
pnpm build && pnpm check && pnpm -r check:types && pnpm vp test run— 326 tests, format and lint clean, typecheck clean across all 5 packages?theme=before any message, 8.4 KB min / 3.3 KB gzip.d.tsresolve through theexportsmap for an external consumerapi.us-e1.tago.io, dashboard created from scratch and every artifact rebuilt from this branch first (see Screenshots)StrictModerequest counting from a clean load: oneGET /sqlfor the mount, onePOST /executeper actionbad_params(it is a 400), and the transport backstop reportsno_responserather than colliding with the host'stimeout. The backstop was observed firing at its 60s deadline with the renamed messageee45b57. Bundle hashes decided the re-test scope: the core IIFE came out byte-identical, so only the React scenario was re-shotSDK metadata merge check) on a real dashboard, bound toCold Chain Truck 04, editing through the widget's owneditDataso the API write, the socket echo and the merge are all the production path. A/B/A across the two bundles: with the fixonRealtimefires again and the cell repaints, with the shipped bundle the API confirmsDevice Data UpdatedandonRealtimenever fires402/408/429mappings are covered by unit tests in that PR, not by live traffic. Manufacturing a real plan cap, execution timeout or rate limit against a live profile would mean abusing itScreenshots
Re-captured after the review fixes, on a Custom Dashboard created from scratch (
6a8306188b06e3000c78b82c) against a local Admin carrying tago-io/admin#5209 and the liveapi.us-e1.tago.io. All three artifacts were rebuilt from the current branch first, so the bundles under test contain the style copy, the coalescing guard and the column-name reader.A purpose-built harness, 13 of 13 assertions green. The three review fixes cannot be shown by the ordinary examples: the host always sends
style: {}, and it always flattenscolumnsto strings. So this shell drives them directly. Style isolation uses two clients in one document, which is the case that actually aliased. The column branch uses the SDK's injectabletargetWindowto answer with{ name, type }columns, so the assertion runs against the really-built bundle rather than a unit-test double. The coalescing row readsrequests 1 -> 3 -> 3: a third identical call after a stop, a restart and a stale settle adds no request.Theme de-duplication, counted rather than asserted. Five unrelated
<html>class mutations plus the host's re-announces produced 13 received theme messages and 0 subscriber calls, because none of them changed the value.Then one real change, the 14th message: exactly 1 subscriber call, and the shell repaints.
Vanilla shell.
sql.listreturned the profile's three saved queries;sql.runreturned 5 device rows in 18 ms, with columns derived from the result.The core merge fix, before and after. Same custom widget, same dashboard, same record on
Cold Chain Truck 04, same button. The widget calls its owneditDatacarryingvalueandtimethrough untouched, so the only real change is insidemetadata. The only thing that differs between the two runs is which build of the widget SDK the page loaded.With the SDK as shipped, the API confirms the write and the widget never hears about it:
onRealtimestays at 1, and the table still readsoccupiedafter the edit setfree.With the fix, the same click produces
onRealtime #2and the cell repaints.value seenis35.8in both shots, which is the point: nothing but metadata moved.React shell. Built to one uploadable file from a consumer project outside the workspace, whose
tsc --noEmitpasses against the generated.d.ts. From a clean load underStrictMode, the network shows oneGET /sqlfor the mount and onePOST /executeper action, so nothing double-fires.Running the disabled saved query surfaces the server's own message through the hook's error state, and the table is gone rather than showing the previous query's rows. It is still
bad_paramsafter tago-io/admin#5209, correctly: an inactive query is a400. That PR split out only402,408and429.Taking this screenshot is what found the bug it now documents. On the first pass the five rows from the previous query were still on screen underneath this error, and the caption called that
useSqlQuerykeepingdataacross a refetch. It was not: the query id had changed, so the shell was rendering one query's rows under another's identity. The docblock only ever promised retention across a refetch of the same query.datanow clears when the identity changes, and switching back to a working query recovers normally.Theme change with
StrictModeon: same rows, same summary, so the theme subscription does not re-run the query.Handoff
One thing this branch cannot finish:
private: truewith no changeset. Writing one now would misfire: changesets still consumes a changeset for a private package, bumping the version and writing a CHANGELOG entry for a release that never publishes. The first changeset belongs in the PR that dropsprivate.Risk (CIA)
Likelihood: 🟢 Low | Impact: 🟢 Low | Exposure: 🟢 Low
New packages only. Nothing publishes, no existing package changes, and no Admin or server change is required.
Related
Reading the protocol surfaced host-side gaps that are fixed separately in tago-io/admin#5209: every 4xx that is not 404/403 collapsed into
bad_params, so a query timeout, a rate limit and a genuine parameter error were indistinguishable by code; and the bridge accepted neitherafter_devicenortest, so a custom dashboard could not paginate a fan-out query.The bridge also discards the column types the API returns, which is left alone for now because changing the
columnsshape breaks any shell already reading it.Closes tago-io/issues#989