Skip to content
This repository was archived by the owner on May 23, 2026. It is now read-only.

refactor: consolidate DevMode blocks with defer, enable skipped test suites - #57

Merged
adnaan merged 1 commit into
mainfrom
defer-script-consolidation
Apr 11, 2026
Merged

refactor: consolidate DevMode blocks with defer, enable skipped test suites#57
adnaan merged 1 commit into
mainfrom
defer-script-consolidation

Conversation

@adnaan

@adnaan adnaan commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Consolidate {{if .lvt.DevMode}} blocks: Add defer to <script> tags and move them from end of <body> into <head>, merging two conditional blocks into one across all 11 templates. defer guarantees execution after DOM parsing — identical behavior to a script at end of body.
  • Enable progressive-enhancement tests: Convert from chromedp.NewExecAllocator (requires local Chrome) to e2etest.StartDockerChrome (portable, works in CI).
  • Enable dialog-patterns tests: Add to WORKING_EXAMPLES in test-all.sh and fix two pre-existing test failures caused by stale polyfill listeners after WebSocket DOM updates.

Test plan

  • All 11 examples pass via ./test-all.sh (up from 9)
  • Verified each template has exactly 1 DevMode block (down from 2)
  • Updated CLAUDE.md boilerplate to match new pattern

🤖 Generated with Claude Code

Move client library <script> from end of <body> into <head> with the
defer attribute, merging two {{if .lvt.DevMode}} blocks into one per
template. Also enable progressive-enhancement and dialog-patterns in
the test suite by converting to Docker Chrome.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 11, 2026 12:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR standardizes how the LiveTemplate client is loaded in example templates (single DevMode block moved to <head> with defer) and re-enables previously skipped E2E suites by switching them to Docker-based Chrome, with accompanying test fixes.

Changes:

  • Consolidate each template’s DevMode conditionals into one block in <head> and load the client via <script defer ...> (removing the duplicated <body> script block).
  • Update progressive-enhancement E2E tests to run against Docker Chrome for CI portability, and add it (plus dialog-patterns) to test-all.sh.
  • Fix dialog-patterns E2E flakiness by resetting page state between subtests.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
ws-disabled/ws-disabled.tmpl Move DevMode client script to <head> with defer; remove duplicated <body> block
todos/todos.tmpl Same DevMode consolidation + defer script change
todos/todos_test.go Capture <html> instead of <body> so head-injected script is included in assertions
test-all.sh Add progressive-enhancement and dialog-patterns to the working suite
shared-notepad/notepad.tmpl Same DevMode consolidation + defer script change
progressive-enhancement/progressive-enhancement.tmpl Same DevMode consolidation + defer script change
progressive-enhancement/progressive_enhancement_test.go Switch E2E to Docker Chrome + restructure HTTP-only tests
login/templates/auth.html Same DevMode consolidation + defer script change
live-preview/preview.tmpl Same DevMode consolidation + defer script change
flash-messages/flash.tmpl Same DevMode consolidation + defer script change
dialog-patterns/dialog-patterns.tmpl Same DevMode consolidation + defer script change
dialog-patterns/dialog_patterns_test.go Navigate fresh between subtests to avoid stale polyfill listeners
counter/counter.tmpl Same DevMode consolidation + defer script change
CLAUDE.md Update documented boilerplate to match the new single-block + defer pattern
chat/chat.tmpl Same DevMode consolidation + defer script change
avatar-upload/avatar-upload.tmpl Same DevMode consolidation + defer script change

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +284 to +288
err = chromedp.Run(ctx,
chromedp.Evaluate(`document.querySelector('dialog#add-dialog input[name="title"]').removeAttribute('required')`, nil),
chromedp.Clear(`dialog#add-dialog input[name="title"]`, chromedp.ByQuery),
chromedp.Click(`dialog#add-dialog button[type="submit"]`, chromedp.ByQuery),
e2etest.WaitFor(`true`, 2*time.Second), // wait for server response

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

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

e2etest.WaitFor('true', 2*time.Second) does not actually wait (the JS expression is immediately true), so this subtest can proceed before the server/WebSocket-driven DOM update has applied. Replace it with a wait condition that reflects the expected post-submit state (e.g., wait for a validation error element/aria-invalid, or for the dialog content to re-render).

Suggested change
err = chromedp.Run(ctx,
chromedp.Evaluate(`document.querySelector('dialog#add-dialog input[name="title"]').removeAttribute('required')`, nil),
chromedp.Clear(`dialog#add-dialog input[name="title"]`, chromedp.ByQuery),
chromedp.Click(`dialog#add-dialog button[type="submit"]`, chromedp.ByQuery),
e2etest.WaitFor(`true`, 2*time.Second), // wait for server response
// Snapshot the dialog content first, then wait for it to change after submit.
err = chromedp.Run(ctx,
chromedp.Evaluate(`document.querySelector('dialog#add-dialog input[name="title"]').removeAttribute('required')`, nil),
chromedp.Evaluate(`window.__addDialogBeforeSubmitHTML = document.getElementById('add-dialog').innerHTML`, nil),
chromedp.Clear(`dialog#add-dialog input[name="title"]`, chromedp.ByQuery),
chromedp.Click(`dialog#add-dialog button[type="submit"]`, chromedp.ByQuery),
e2etest.WaitFor(`document.getElementById('add-dialog') && document.getElementById('add-dialog').innerHTML !== window.__addDialogBeforeSubmitHTML`, 2*time.Second),

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +56
for i := 0; i < 50; i++ {
resp, err := http.Get(addr)

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

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

The server readiness loop uses http.Get with the default client (no timeout). If a connection attempt stalls, this can hang the test process. Use an http.Client{Timeout: ...} (or context.WithTimeout) for the probe requests.

Suggested change
for i := 0; i < 50; i++ {
resp, err := http.Get(addr)
probeClient := &http.Client{Timeout: 2 * time.Second}
for i := 0; i < 50; i++ {
resp, err := probeClient.Get(addr)

Copilot uses AI. Check for mistakes.
Comment on lines +618 to +620
// Wait for server
for i := 0; i < 50; i++ {
resp, err := http.Get(url)

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

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

This readiness probe also uses http.Get without a timeout. To avoid potential hangs in CI, use an http.Client with a short timeout (or context) for the loop’s GET requests.

Suggested change
// Wait for server
for i := 0; i < 50; i++ {
resp, err := http.Get(url)
readinessClient := &http.Client{Timeout: 500 * time.Millisecond}
// Wait for server
for i := 0; i < 50; i++ {
resp, err := readinessClient.Get(url)

Copilot uses AI. Check for mistakes.
@adnaan
adnaan force-pushed the defer-script-consolidation branch from e51b2a2 to 9bebfbb Compare April 11, 2026 12:56
@adnaan
adnaan merged commit 2ae4cd3 into main Apr 11, 2026
23 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants