chore(example): add polling/streaming mode toggle and status panel by duyhungtnn · Pull Request #349 · bucketeer-io/javascript-client-sdk

duyhungtnn · 2026-09-08T02:23:07Z

This pull request improves the example application for the Bucketeer JS Client SDK: a simple polling/streaming toggle, environment-variable-driven configuration, and clearer documentation.

Example app improvements:

The example app UI (example/index.html) has a new header, a mode toggle (polling vs. built-in streaming), a "Clear log" button, and a panel showing the current flag value. Button states are more accurately managed during initialization.
The app now reads feature flag and goal identifiers, plus the streaming toggle, from environment variables, configurable via .env. Defaults are provided in example/env.template.
example/index.ts wires the mode toggle to the SDK's enableStreaming config option, refreshes the displayed flag value on evaluation updates, and locks the mode radios during initialization to avoid a race between a mode change and an in-flight initializeBKTClient call.

Documentation and configuration:

The README now distinguishes between the main .env (for development/tests) and the example app's own .env, with setup instructions for both.
example/env.template documents the demo environment variables, including the streaming toggle and demo flag/goal IDs.
CI now runs the SDK build before the type-check step.

Copilot

🟡 Changes recommended

Moderate issues remain in SSE lifecycle handling, status accuracy, mode switching, and accessibility.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds runtime-selectable polling/SSE modes and network-derived connection diagnostics to the example app.

Changes:

Adds mode controls, connection status metrics, and flag-value UI.
Adds fetch instrumentation and a custom SSE adapter.
Separates example environment configuration from SDK development settings.

File summaries

File	Description
`README.md`	Documents standalone example environment setup.
`example/vite.config.ts`	Loads environment variables from the example directory.
`example/types.ts`	Defines instrumentation and transport types.
`example/package.json`	Adds the custom SSE client dependency.
`example/loggingFetch.ts`	Instruments polling and streaming activity.
`example/index.ts`	Implements mode selection and status management.
`example/index.html`	Adds mode controls and status panels.
`example/eventSourceAdapter.ts`	Adapts `eventsource-client` to the SDK interface.
`example/env.template`	Provides example-specific configuration variables.

Review details

Files reviewed: 9/10 changed files
Comments generated: 9
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

Two moderate error-handling and terminal-stream-state issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 9/10 changed files
Comments generated: 2
Review effort level: Balanced

A Copilot review on PR #349 found that the custom EventSourceAdapter and loggingFetch instrumentation could report the stream as open or connecting when it had actually failed or closed: eventsource-client fires onConnect for any resolved response (even 401/403) and skips onDisconnect whenever the fetch itself rejects. It also flagged a mode-switch race during init, a stale timestamp left over after re-init, a dead README link, and a missing status role for screen readers.

A second Copilot review on PR #349 found two more gaps: building BKTConfig/BKTUser outside the init try-block meant a synchronous validation error (e.g. a malformed endpoint) left Initialize and the mode radios stuck disabled forever, and a successful stream response with no body was reported the same as an ordinary disconnect even though the SDK treats it as a permanent, non-retryable failure.

Copilot

🟡 Changes recommended

Multiple moderate SSE parsing, lifecycle, and accessibility issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

example/eventSourceAdapter.ts:6

This React Native example is inaccurate: eventsource-client@1.2.0 itself requires ReadableStream, and its default entry point rejects bodies that are not Web ReadableStream instances. Remove this claimed use case or use an adapter/library that actually supports that runtime without the polyfill.

// the built-in transport can't work (e.g. React Native without a fetch +
// ReadableStream polyfill) or a specific SSE library is required.

example/index.html:24

This wording implies that the SDK's default polling interval is 60 seconds, but the SDK default is 10 minutes; only this example overrides it to 60 seconds. Distinguish the default mode from the demo-specific interval.

      <label style="display: block;"><input type="radio" name="mode" value="polling"> polling (comparison mode, matches the SDK default: up to 60s to see a change)</label>

Files reviewed: 9/10 changed files
Comments generated: 5
Review effort level: Balanced

A third Copilot review on PR #349 found the custom EventSourceAdapter still reported an unreadable-body stream response as recoverable instead of terminal like the built-in transport, dropped an explicit "event: message" block because StreamConnection never registers a listener under that name, made every heartbeat re-announce the whole status panel to screen readers, mis-stated the SDK's default polling interval, and could split or silently drop the logging instrumentation's own CRLF and mid-stream failure handling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

A third Copilot review on PR #349 found the custom EventSourceAdapter still reported an unreadable-body stream response as recoverable instead of terminal like the built-in transport, dropped an explicit "event: message" block because StreamConnection never registers a listener under that name, made every heartbeat re-announce the whole status panel to screen readers, mis-stated the SDK's default polling interval, and could split or silently drop the logging instrumentation's own CRLF and mid-stream failure handling.

Copilot

🟡 Changes recommended

The new SSE adapter and framing parser lack required automated coverage for critical edge cases.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

example/loggingFetch.ts:52

This rescans and reallocates the entire unterminated SSE block on every network chunk. Large initial put snapshots split across many chunks therefore make the display-side tee branch quadratic and can stall the page even though the SDK parser avoids this with a retained search offset (src/internal/streaming/FetchEventSource.ts:140-174). Parse completed delimiters incrementally and retain only the unconsumed suffix.

      buffer += chunkText.replace(/\r\n/g, '\n').replace(/\r/g, '\n')
      const blocks = buffer.split('\n\n')
      buffer = blocks.pop() ?? ''

Files reviewed: 9/10 changed files
Comments generated: 2
Review effort level: Balanced

Copilot

🟢 Approval recommended

All reviewed changes are covered with no unresolved approval-blocking issues.

Review details

Files reviewed: 13/14 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

🟡 Changes recommended

SSE data parsing incorrectly removes all leading whitespace from payload lines.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 13/14 changed files
Comments generated: 1
Review effort level: Balanced

Copilot

🔵 Needs a closer look

Two moderate status-reporting issues must be corrected before approval.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

example/eventSourceAdapter.ts:172

Report the closure before invoking onerror. For non-terminal statuses outside the fast-retry set (for example 400/413/422), onerror synchronously starts the SDK's polling fallback, whose request sets the panel to polling fallback; the subsequent closed report here overwrites it with disconnected, leaving the custom-mode status incorrect while polling is active.
example/index.ts:136
Every streaming initialization still performs the SDK's mandatory bootstrap get_evaluations request (BKTClient.scheduleAndFetch() starts tasks and then calls fetchEvaluations). This branch therefore labels a normally connecting stream as polling fallback before any streaming failure has occurred; with a slow stream handshake, the incorrect state can remain visible until the connection opens or times out. Ignore this bootstrap request while initializing is true, and reserve the fallback label for later polling requests.

Files reviewed: 13/14 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

🟡 Changes recommended

The fallback polling status can be incorrectly suppressed during initialization.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 13/14 changed files
Comments generated: 1
Review effort level: Balanced

The example wrapped fetch to re-implement SSE framing (block splitting, CR handling across chunk boundaries, data-line stripping) purely to feed a status panel counting heartbeats and events. An app developer only needs to see how to enable streaming and configure it, so the panel, the instrumented fetch and its spec are gone. The custom EventSource adapter stays as the example of replacing the SSE transport, now a plain EventSourceLike with no reporter threaded through it. Its comment handler keeps the bare onmessage tick, which the SDK watchdog depends on to avoid tearing down a quiet stream.

The example only needs to show how to turn streaming on. The custom transport adapter and its spec re-implemented SSE handling that the SDK already owns, so the demo now offers a plain polling/streaming toggle driving enableStreaming, and eventsource-client is no longer needed.

so the example could linked to the correct lib and type check will run correctly with the example setup

Copilot

🟡 Changes recommended

The implemented UI omits the advertised custom adapter mode and streaming status panel.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Balanced

Copilot

🟡 Changes recommended

The flag panel retains stale values across lifecycle changes and does not announce asynchronous updates accessibly.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 7/7 changed files
Comments generated: 2
Review effort level: Balanced

…ibly Destroying the client, or a reinitialization that fails, previously left the last flag value on screen even though no client exists anymore. Also mark the flag value span with role="status" so screen readers announce updates from initialization and evaluation-update events.

Copilot

🟢 Approval recommended

The changes consistently implement the documented example workflow without identified correctness issues.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

🔵 Needs a closer look

The setup instructions omit replacing the placeholder feature and goal identifiers required for the example to work.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

README.md:24

Copying the template and updating only the API credentials leaves VITE_BKT_FEATURE_TAG, VITE_BKT_FEATURE_ID, and VITE_BKT_GOAL_ID at placeholder values, so the documented example will not evaluate the user's flag or track their goal. Explicitly instruct users to replace those identifiers as well.

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

Copilot

🟢 Approval recommended

The reviewed changes are coherent, and previously identified correctness, accessibility, and test-coverage issues have been addressed.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Balanced

duyhungtnn · 2026-09-18T14:59:45Z

Hi @t-kikuc, I updated this example to make it simpler.
Please take a look.

The setup steps never told the reader to set VITE_BKT_FEATURE_TAG, VITE_BKT_FEATURE_ID or VITE_BKT_GOAL_ID, and env.template marked them optional. Left at the shipped placeholders, the SDK requests a feature tag that has no flags, so the server returns no evaluations: the displayed value stays at the default and the update listener never fires. The SSE walkthrough then shows nothing, and the reader cannot tell whether SSE is broken or their own setup is.

duyhungtnn requested a balanced review from Copilot September 8, 2026 15:10

Copilot started reviewing on behalf of duyhungtnn September 8, 2026 15:11

View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

duyhungtnn requested a balanced review from Copilot September 8, 2026 16:13

Copilot started reviewing on behalf of duyhungtnn September 8, 2026 16:13

View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread example/index.ts Outdated

Comment thread example/loggingFetch.ts Outdated

duyhungtnn requested a balanced review from Copilot September 8, 2026 16:46

Copilot started reviewing on behalf of duyhungtnn September 8, 2026 16:46

View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread example/eventSourceAdapter.ts Outdated

Comment thread example/eventSourceAdapter.ts Outdated

Comment thread example/index.html Outdated

Comment thread example/loggingFetch.ts Outdated

Comment thread example/loggingFetch.ts Outdated

duyhungtnn force-pushed the chore/example-see-update branch 2 times, most recently from 35d5710 to d1a0fc2 Compare September 8, 2026 17:20

duyhungtnn requested a balanced review from Copilot September 8, 2026 17:25

Copilot started reviewing on behalf of duyhungtnn September 8, 2026 17:25

View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread example/eventSourceAdapter.ts Outdated

Comment thread example/loggingFetch.ts Outdated

duyhungtnn requested a balanced review from Copilot September 9, 2026 01:55

Copilot started reviewing on behalf of duyhungtnn September 9, 2026 01:56

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

duyhungtnn requested a balanced review from Copilot September 9, 2026 02:19

Copilot started reviewing on behalf of duyhungtnn September 9, 2026 02:20

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread example/loggingFetch.ts Outdated

duyhungtnn requested a balanced review from Copilot September 9, 2026 02:40

Copilot started reviewing on behalf of duyhungtnn September 9, 2026 02:40

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

duyhungtnn requested a balanced review from Copilot September 9, 2026 02:54

Copilot started reviewing on behalf of duyhungtnn September 9, 2026 02:55

View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread example/index.ts Outdated

duyhungtnn added 3 commits September 18, 2026 16:00

ci: build lib first before run type check

f14cdeb

so the example could linked to the correct lib and type check will run correctly with the example setup

duyhungtnn force-pushed the chore/example-see-update branch from ff11e53 to f14cdeb Compare September 18, 2026 09:02

chore: restore pnpm-lock.yaml

968c144

duyhungtnn requested a balanced review from Copilot September 18, 2026 09:11

Copilot started reviewing on behalf of duyhungtnn September 18, 2026 09:11

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread example/index.html

Comment thread example/index.html Outdated

duyhungtnn requested a balanced review from Copilot September 18, 2026 09:22

Copilot started reviewing on behalf of duyhungtnn September 18, 2026 09:23

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread example/index.ts

Comment thread example/index.html Outdated

duyhungtnn requested a balanced review from Copilot September 18, 2026 13:56

Copilot started reviewing on behalf of duyhungtnn September 18, 2026 13:56

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

docs: add README steps for trying real-time flag updates via SSE

7764670

duyhungtnn requested a balanced review from Copilot September 18, 2026 14:37

Copilot started reviewing on behalf of duyhungtnn September 18, 2026 14:38

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

duyhungtnn requested review from t-kikuc and a balanced review from Copilot September 18, 2026 14:56

Copilot started reviewing on behalf of duyhungtnn September 18, 2026 14:57

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

duyhungtnn marked this pull request as ready for review September 18, 2026 14:58

t-kikuc previously approved these changes Sep 28, 2026

View reviewed changes

Comment thread README.md Outdated

duyhungtnn dismissed t-kikuc’s stale review via 4009ee5 October 9, 2026 05:19

duyhungtnn requested review from cre8ivejp and t-kikuc October 9, 2026 05:22

Conversation

duyhungtnn commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

Uh oh!

duyhungtnn commented Sep 8, 2026 •

edited

Loading