Add usermod web UI injection mechanism (/um.js)#5741
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughWLED adds usermod UI-injection hooks, aggregates generated JavaScript into a new ChangesUsermod UI injection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant WLEDServer
participant UsermodManager
participant Usermods
Browser->>WLEDServer: GET /um.js
WLEDServer->>UsermodManager: addUIInjectCode(dest)
UsermodManager->>Usermods: addUIInjectCode(dest)
Usermods-->>UsermodManager: JavaScript injection code
UsermodManager-->>WLEDServer: Aggregated JavaScript
WLEDServer-->>Browser: application/javascript response
sequenceDiagram
participant WLEDUI
participant WLEDServer
participant umInject
WLEDUI->>WLEDUI: Receive json.info.u
WLEDUI->>WLEDServer: Load /um.js once
WLEDServer-->>WLEDUI: Define umInject(s)
WLEDUI->>WLEDUI: Render state with readState(s)
WLEDUI->>umInject: Safely invoke umInject(s)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wled00/data/index.js`:
- Line 1624: Wrap the umInject(s) call in the usermod injection check within a
try...catch so exceptions from injected JavaScript cannot escape readState().
Preserve the existing function-type guard, and handle the caught error locally
without disrupting requestJson() or triggering its retry behavior.
- Line 1823: Update the usermod injection check in the surrounding dashboard
initialization to use logical checks instead of optional chaining, preserving
the condition that usermod data exists. Call loadUmInject with the current state
object s so it does not perform a redundant fetch during script loading.
- Around line 232-240: Update loadUmInject to accept the current state s, and in
its script onload handler invoke umInject(s) directly when available instead of
calling requestJson(). Wrap the usermod invocation in try/catch so injected
exceptions are contained and do not interrupt readState(); regenerate the web UI
headers by running npm run build after editing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6472d9b7-57e0-4787-90fd-dcd4ebb1361c
📒 Files selected for processing (4)
wled00/data/index.jswled00/fcn_declare.hwled00/um_manager.cppwled00/wled_server.cpp
- umInjectSafe() wraps umInject() in try/catch so an exception in usermod-provided JS cannot abort readState() and trigger requestJson()'s retry loop - pass the current state into loadUmInject(s) and run the first injection directly on script load instead of refetching /json/si - use json.info && json.info.u to match the surrounding style
…Safe shield, no redundant fetch)
|
I guess that's 3 of us working on this feature now! I would vote against this approach entirely, though. Usermod UI assembly and injection should be done at compile time, so it can also leverage the zipping and minification features. Runtime assembly also opens a number of memory management problems that can be entirely avoided. I've been working on a manifest-based system, though I haven't got to the main UI reparse points yet. I've pushed it up what's available now as PR #5742 . Maybe we could integrate the main UI injection hook to run at build time? |
|
Usermod needs to inject variable elements at runtime. So... I'm going to watch closely how you'll manage to do that at compile time. On the other hand, nothing is prohibiting usermod to include pre-minimised and compressed web page header and (then) register a web request handler right now. For the later see my Multi relay usermod. The footprint and complexity of this approach is minimal and all that is needed additionally are some support JS functions and DOM objects so that usermod developer can leverage some work. |
For the overwhelming majority of cases I expect the UI code to be known and provisioned at compile time, and the relevant data to populate it to be exposed via The PowerManager usermod described here is a good example. The JS code to add is 100% static ( |
|
I am not talking about HTML elements. I am talking about the usermod's decision to inject anything at all (depending on states like enabled, features used/enabled, etc). These can't all be determined at compile time. You would be increasing UI code unnecessarily with no benefit if you would compile-in everything. Don't judge everything just by one example. See also the I do understand you are working on the idea that each usermod will supply its own configuration HTML (or something similar, which a usermod could do right now by providing its own web handler). But that is still far in the future as is eventual pre-compiled UI injection. There is not even a POC available for the latter while this PR does it at minimal impact and no side effects. However, that's just my 2c as I have no benefit of either solution. I just tried to help a person that has been nice to me but unfortunately has no coding experience. |
If there is a usermod that wanted to decide at runtime whether to serve additional code, it could inject a (And - to be completely frank - I don't think that will ever come up. In virtually every case I can think of, we're best served to always include the usermod JS code as part of the initial UI load, and push the job of deciding what to render on to the client browser.)
Oh for sure, adding common features to the main UI is a good idea. The minification process can strip out those not used in a particular build.
I strongly disagree with "minimal impact" -- this approach is inefficient at runtime (an extra RTT in every case, used or not); expensive on RAM (using Your heart is in the right place, this is a feature we all want. I think we can build on the ideas we've got to make a better version. |
|
Just chiming in, I, as is probably known, don't have a lot of programming knowledge, was just in need of functionality with my PowerManager usermod. So I don't have much say in how this should be implemented, I just ran into this issue and likely more mods in the future would like similar functionality (the ability to add things in the WLED GUI from within a usermod). So for right now, I don't have too much to add. For now I will use this method in my own build for the boards that require it (currently the Dig-Next-2, more to follow though) but once an official way becomes available I'll happily switch over! |
3 participants
Adds a small, usermod-agnostic mechanism that lets usermods inject their own elements into the main web UI without patching
index.js— based on the design proposed by @blazoncek.How it works
virtual void addUIInjectCode(Print &dest)and print plain JavaScript./um.js(no-store), wrapping all usermods' output infunction umInject(s){...}./um.jsonce (loadUmInject(), triggered when a state response carriesinfo.u, i.e. usermods are present) and callsumInject(s)at the end of everyreadState(s)— so injected elements survive UI re-renders (e.g.populateSegments()), for both fetch and WebSocket updates./um.jsserves an empty function body and the UI is unchanged; if the script fails to load,onerrorlogs and the UI continues normally.Deltas from the original proposal
umInject(s)receives the freshly applied state object — injected UI usually renders usermod data carried in the state (viaaddToJsonState()), and passing it avoids a second fetch.loadUmInject()is guarded against double-insertion of the script tag.onloadtriggers arequestJson()so the first injection runs with state available (the script finishes loading after the initial render).WLED_ENABLE_UM_UI_INJECTcapability macro next to the virtual, so external usermods can#ifdef-guard their override and keep compiling against WLED bases that don't have the mechanism (yet).Tested
Verified on physical hardware — a QuinLED Dig-Next-2 (ESP32) — with the PowerManager usermod serving a segment-card menu through this mechanism: injection runs after every render (fetch and WS), re-renders keep injected elements, and stock behavior is untouched when no usermod provides inject code.
esp32devbuilds clean; generatedhtml_*.hfiles are intentionally not included.What this allows in practice — a usermod-provided menu living right on the segment card, with zero PowerManager-specific code in
index.js:One practical note for usermod authors that surfaced during testing: injected code runs on the main UI page, where only its globals (
d,gId,requestJson, ...) exist — settings-page helpers likecE()are not available.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/um.jsendpoint to serve dynamic usermod UI enhancements.Bug Fixes