Skip to content

feat: Add liveReload#1407

Open
matz3 wants to merge 3 commits into
mainfrom
feat/liveReload
Open

feat: Add liveReload#1407
matz3 wants to merge 3 commits into
mainfrom
feat/liveReload

Conversation

@matz3

@matz3 matz3 commented Jun 8, 2026

Copy link
Copy Markdown
Member

JIRA: CPOUI5FOUNDATION-1224

@matz3

matz3 commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

To be decided: Should clients try to reconnect to the server, e.g. in case it has been restarted? This is currently not implemented, but I think it should be added.

@matz3 matz3 force-pushed the feat/liveReload branch from 8f5a612 to 2cd5e90 Compare June 9, 2026 09:41
@matz3

matz3 commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

To be decided: Should clients try to reconnect to the server, e.g. in case it has been restarted? This is currently not implemented, but I think it should be added.

This has now been implemented.

@matz3 matz3 force-pushed the feat/liveReload branch 3 times, most recently from ff657e6 to 409ab00 Compare June 9, 2026 11:49
@matz3 matz3 marked this pull request as ready for review June 9, 2026 11:58
@matz3 matz3 requested a review from a team June 9, 2026 11:58

@d3xter666 d3xter666 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

with small comments

Comment thread packages/cli/lib/cli/commands/serve.js
Comment thread packages/server/lib/liveReload/client.js Outdated
@d3xter666 d3xter666 requested a review from a team June 9, 2026 12:56
@matz3 matz3 force-pushed the feat/liveReload branch from 409ab00 to 7fd70bc Compare June 9, 2026 13:48
@matz3 matz3 requested a review from d3xter666 June 9, 2026 13:48
Comment thread internal/documentation/docs/pages/Server.md Outdated
Comment thread internal/documentation/docs/pages/Server.md Outdated
Comment thread internal/documentation/docs/updates/migrate-v5.md Outdated
Comment thread packages/server/lib/liveReload/client.js Outdated
Comment thread packages/server/lib/liveReload/client.js
Comment thread packages/server/lib/liveReload/server.js
Comment thread packages/server/lib/middleware/MiddlewareManager.js
matz3 added 3 commits June 9, 2026 16:53
Adds optional live reload support to the server:
- New `serveLiveReloadClient` middleware serves the client at
  `/.ui5/liveReload/client.js`.
- The `serveResources` middleware injects the client script tag into
  HTML responses when `liveReload` is enabled.
- A WebSocket server attaches to the HTTP server and notifies connected
  clients when the BuildServer emits a `sourcesChanged` event.

The feature is opt-in via the new `liveReload` option to `serve()` and
defaults to `false`.

JIRA: CPOUI5FOUNDATION-1224
- Add new `server.settings.liveReload` boolean option to the project
  configuration schema, available with specVersion 5.0 and higher.
- BuildServer now emits a debounced `sourcesChanged` event (100ms
  debounce) whenever watched source files change, so a burst of changes
  results in a single notification.

JIRA: CPOUI5FOUNDATION-1224
- Add new `--live-reload` CLI flag (defaults to true) for the
  `ui5 serve` command. Pass `--no-live-reload` to disable it.
- The flag overrides the `server.settings.liveReload` setting in the
  project's `ui5.yaml`.
- When neither the CLI flag nor the configuration sets a value, live
  reload is enabled by default.

JIRA: CPOUI5FOUNDATION-1224
@matz3 matz3 force-pushed the feat/liveReload branch from 7fd70bc to 77e4a42 Compare June 9, 2026 14:55
@matz3 matz3 requested a review from RandomByte June 9, 2026 14:56
@@ -0,0 +1,108 @@
(function() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No unit tests for the client code 😿

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to introduce real browser tests as I think it might be an overkill. But as it's based on WebSockets, my idea would be to mock the browser deps and run it in Node.js, as we can also connect via WebSocket from there. This would also make the coverage measurements much simpler.

// const changedResourcePaths = [...changes.values()].flat();
// this.emit("sourcesChanged", changedResourcePaths);
// Debounced emit so a burst of file changes results in a single reload notification
if (this.#sourcesChangedTimeout) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we were talking about this behavior today: Do we have unit tests for this change and the debounce in particular?

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.

3 participants