Skip to content

Commit 66f5a8e

Browse files
Brooooooklynclaude
andcommitted
fix review: remove synthetic watcher emit, fix test setup
- Remove server.watcher.emit('change', file) for component resources: Vite treats HTML changes with no module graph entries as full reloads, which would regress template HMR behavior. - Fix test to call config() with command='serve' so watchMode=true and resourceToComponent is actually populated during transform. Tests now use real temp files and assert exact return values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6b07c3b commit 66f5a8e

File tree

2 files changed

+146
-101
lines changed

2 files changed

+146
-101
lines changed

napi/angular-compiler/test/hmr-hot-update.test.ts

Lines changed: 143 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,37 @@
99
* HMR updates for global stylesheets and prevented PostCSS/Tailwind from
1010
* processing changes.
1111
*/
12-
import type { Plugin, ModuleNode, ViteDevServer, HmrContext } from 'vite'
13-
import { describe, it, expect, vi } from 'vitest'
12+
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
13+
import { tmpdir } from 'node:os'
14+
import { join } from 'node:path'
15+
16+
import type { Plugin, ModuleNode, HmrContext } from 'vite'
1417
import { normalizePath } from 'vite'
18+
import { afterAll, beforeAll, describe, it, expect, vi } from 'vitest'
1519

1620
import { angular } from '../vite-plugin/index.js'
1721

22+
let tempDir: string
23+
let appDir: string
24+
let templatePath: string
25+
let stylePath: string
26+
27+
beforeAll(() => {
28+
tempDir = mkdtempSync(join(tmpdir(), 'hmr-test-'))
29+
appDir = join(tempDir, 'src', 'app')
30+
mkdirSync(appDir, { recursive: true })
31+
32+
templatePath = join(appDir, 'app.component.html')
33+
stylePath = join(appDir, 'app.component.css')
34+
35+
writeFileSync(templatePath, '<h1>Hello</h1>')
36+
writeFileSync(stylePath, 'h1 { color: red; }')
37+
})
38+
39+
afterAll(() => {
40+
rmSync(tempDir, { recursive: true, force: true })
41+
})
42+
1843
function getAngularPlugin() {
1944
const plugin = angular({ liveReload: true }).find(
2045
(candidate) => candidate.name === '@oxc-angular/vite',
@@ -28,20 +53,16 @@ function getAngularPlugin() {
2853
}
2954

3055
function createMockServer() {
31-
const watchedFiles = new Set<string>()
32-
const unwatchedFiles = new Set<string>()
3356
const wsMessages: any[] = []
34-
const emittedEvents: { event: string; path: string }[] = []
57+
const unwatchedFiles = new Set<string>()
3558

3659
return {
3760
watcher: {
3861
unwatch(file: string) {
3962
unwatchedFiles.add(file)
4063
},
4164
on: vi.fn(),
42-
emit(event: string, path: string) {
43-
emittedEvents.push({ event, path })
44-
},
65+
emit: vi.fn(),
4566
},
4667
ws: {
4768
send(msg: any) {
@@ -57,11 +78,10 @@ function createMockServer() {
5778
use: vi.fn(),
5879
},
5980
config: {
60-
root: '/test',
81+
root: tempDir,
6182
},
6283
_wsMessages: wsMessages,
6384
_unwatchedFiles: unwatchedFiles,
64-
_emittedEvents: emittedEvents,
6585
}
6686
}
6787

@@ -79,20 +99,89 @@ function createMockHmrContext(
7999
} as HmrContext
80100
}
81101

102+
async function callPluginHook<TArgs extends unknown[], TResult>(
103+
hook:
104+
| {
105+
handler: (...args: TArgs) => TResult
106+
}
107+
| ((...args: TArgs) => TResult)
108+
| undefined,
109+
...args: TArgs
110+
): Promise<TResult | undefined> {
111+
if (!hook) return undefined
112+
if (typeof hook === 'function') return hook(...args)
113+
return hook.handler(...args)
114+
}
115+
116+
/**
117+
* Set up a plugin through the full Vite lifecycle so that internal state
118+
* (watchMode, viteServer, resourceToComponent, componentIds) is populated.
119+
*/
120+
async function setupPluginWithServer(plugin: Plugin) {
121+
const mockServer = createMockServer()
122+
123+
// config() sets watchMode = true when command === 'serve'
124+
await callPluginHook(
125+
plugin.config as Plugin['config'],
126+
{} as any,
127+
{
128+
command: 'serve',
129+
mode: 'development',
130+
} as any,
131+
)
132+
133+
// configResolved() stores the resolved config
134+
await callPluginHook(
135+
plugin.configResolved as Plugin['configResolved'],
136+
{
137+
build: {},
138+
isProduction: false,
139+
} as any,
140+
)
141+
142+
// configureServer() sets up the custom watcher and stores viteServer
143+
if (typeof plugin.configureServer === 'function') {
144+
await (plugin.configureServer as Function)(mockServer)
145+
}
146+
147+
return mockServer
148+
}
149+
150+
/**
151+
* Transform a component that references external template + style files,
152+
* populating resourceToComponent and componentIds.
153+
*/
154+
async function transformComponent(plugin: Plugin) {
155+
const componentFile = join(appDir, 'app.component.ts')
156+
const componentSource = `
157+
import { Component } from '@angular/core';
158+
159+
@Component({
160+
selector: 'app-root',
161+
templateUrl: './app.component.html',
162+
styleUrls: ['./app.component.css'],
163+
})
164+
export class AppComponent {}
165+
`
166+
167+
if (!plugin.transform || typeof plugin.transform === 'function') {
168+
throw new Error('Expected plugin transform handler')
169+
}
170+
171+
await plugin.transform.handler.call(
172+
{ error() {}, warn() {} } as any,
173+
componentSource,
174+
componentFile,
175+
)
176+
}
177+
82178
describe('handleHotUpdate - Issue #185', () => {
83179
it('should let non-component CSS files pass through to Vite HMR', async () => {
84180
const plugin = getAngularPlugin()
181+
await setupPluginWithServer(plugin)
85182

86-
// Configure the plugin (sets up internal state)
87-
if (plugin.configResolved && typeof plugin.configResolved !== 'function') {
88-
throw new Error('Expected configResolved to be a function')
89-
}
90-
if (typeof plugin.configResolved === 'function') {
91-
await plugin.configResolved({ build: {}, isProduction: false } as any)
92-
}
93-
94-
// Call handleHotUpdate with a global CSS file (not a component resource)
95-
const globalCssFile = normalizePath('/workspace/src/styles.css')
183+
// A global CSS file (not referenced by any component's styleUrls)
184+
const globalCssFile = normalizePath(join(tempDir, 'src', 'styles.css'))
96185
const mockModules = [{ id: globalCssFile, type: 'css' }]
97186
const ctx = createMockHmrContext(globalCssFile, mockModules)
98187

@@ -101,113 +190,75 @@ describe('handleHotUpdate - Issue #185', () => {
101190
result = await plugin.handleHotUpdate(ctx)
102191
}
103192

104-
// Non-component CSS should NOT be swallowed - result should be undefined
105-
// (pass through) or the original modules, NOT an empty array
193+
// Non-component CSS should NOT be swallowed — either undefined (pass through)
194+
// or the original modules array, but NOT an empty array
106195
if (result !== undefined) {
107-
expect(result.length).toBeGreaterThan(0)
196+
expect(result).toEqual(mockModules)
108197
}
109-
// If result is undefined, Vite uses ctx.modules (the default), which is correct
110198
})
111199

112-
it('should return [] for component resource files that are managed by custom watcher', async () => {
200+
it('should return [] for component CSS files managed by custom watcher', async () => {
113201
const plugin = getAngularPlugin()
114-
const mockServer = createMockServer()
202+
const mockServer = await setupPluginWithServer(plugin)
203+
await transformComponent(plugin)
115204

116-
// Set up the plugin's internal state by going through the lifecycle
117-
if (typeof plugin.configResolved === 'function') {
118-
await plugin.configResolved({ build: {}, isProduction: false } as any)
119-
}
205+
// The component's CSS file IS in resourceToComponent
206+
const componentCssFile = normalizePath(stylePath)
207+
const mockModules = [{ id: componentCssFile }]
208+
const ctx = createMockHmrContext(componentCssFile, mockModules, mockServer)
120209

121-
// Call configureServer to set up the custom watcher infrastructure
122-
if (typeof plugin.configureServer === 'function') {
123-
await (plugin.configureServer as Function)(mockServer)
210+
let result: ModuleNode[] | void | undefined
211+
if (typeof plugin.handleHotUpdate === 'function') {
212+
result = await plugin.handleHotUpdate(ctx)
124213
}
125214

126-
// Now we need to transform a component to populate resourceToComponent.
127-
// Transform a component that references an external template
128-
const componentSource = `
129-
import { Component } from '@angular/core';
130-
131-
@Component({
132-
selector: 'app-root',
133-
templateUrl: './app.component.html',
134-
styleUrls: ['./app.component.css'],
135-
})
136-
export class AppComponent {}
137-
`
138-
139-
if (!plugin.transform || typeof plugin.transform === 'function') {
140-
throw new Error('Expected plugin transform handler')
141-
}
215+
// Component resources MUST be swallowed (return [])
216+
expect(result).toEqual([])
217+
})
142218

143-
// Transform the component to populate internal maps
144-
// Note: This may fail if the template/style files don't exist, but it should
145-
// still register the resource paths in resourceToComponent during dependency resolution
146-
try {
147-
await plugin.transform.handler.call(
148-
{
149-
error() {},
150-
warn() {},
151-
} as any,
152-
componentSource,
153-
'/workspace/src/app/app.component.ts',
154-
)
155-
} catch {
156-
// Transform may fail because template files don't exist on disk,
157-
// but resourceToComponent should still be populated
158-
}
219+
it('should return [] for component template HTML files managed by custom watcher', async () => {
220+
const plugin = getAngularPlugin()
221+
const mockServer = await setupPluginWithServer(plugin)
222+
await transformComponent(plugin)
159223

160-
// Test handleHotUpdate with a component resource file
161-
const componentCssFile = normalizePath('/workspace/src/app/app.component.css')
162-
const ctx = createMockHmrContext(componentCssFile, [{ id: componentCssFile }], mockServer)
224+
// The component's HTML template IS in resourceToComponent
225+
const componentHtmlFile = normalizePath(templatePath)
226+
const ctx = createMockHmrContext(componentHtmlFile, [{ id: componentHtmlFile }], mockServer)
163227

164228
let result: ModuleNode[] | void | undefined
165229
if (typeof plugin.handleHotUpdate === 'function') {
166230
result = await plugin.handleHotUpdate(ctx)
167231
}
168232

169-
// Component resources SHOULD be swallowed (return []) because they're handled
170-
// by the custom fs.watch. If the transform didn't populate resourceToComponent
171-
// (because the files don't exist), the result might pass through - that's also
172-
// acceptable since Vite's default handling would apply.
173-
// The key assertion is in the first test: non-component files must NOT be swallowed.
174-
if (result !== undefined) {
175-
// Either empty (swallowed) or passed through
176-
expect(Array.isArray(result)).toBe(true)
177-
}
233+
// Component templates MUST be swallowed (return [])
234+
expect(result).toEqual([])
178235
})
179236

180237
it('should not swallow non-resource HTML files', async () => {
181238
const plugin = getAngularPlugin()
239+
await setupPluginWithServer(plugin)
182240

183-
if (typeof plugin.configResolved === 'function') {
184-
await plugin.configResolved({ build: {}, isProduction: false } as any)
185-
}
186-
187-
// An HTML file that is NOT a component template (e.g., index.html)
188-
const indexHtml = normalizePath('/workspace/index.html')
189-
const ctx = createMockHmrContext(indexHtml, [{ id: indexHtml }])
241+
// index.html is NOT a component template
242+
const indexHtml = normalizePath(join(tempDir, 'index.html'))
243+
const mockModules = [{ id: indexHtml }]
244+
const ctx = createMockHmrContext(indexHtml, mockModules)
190245

191246
let result: ModuleNode[] | void | undefined
192247
if (typeof plugin.handleHotUpdate === 'function') {
193248
result = await plugin.handleHotUpdate(ctx)
194249
}
195250

196-
// Non-component HTML files should pass through
251+
// Non-component HTML should pass through, not be swallowed
197252
if (result !== undefined) {
198-
expect(result.length).toBeGreaterThan(0)
253+
expect(result).toEqual(mockModules)
199254
}
200255
})
201256

202257
it('should pass through non-style/template files unchanged', async () => {
203258
const plugin = getAngularPlugin()
259+
await setupPluginWithServer(plugin)
204260

205-
if (typeof plugin.configResolved === 'function') {
206-
await plugin.configResolved({ build: {}, isProduction: false } as any)
207-
}
208-
209-
// A .ts file that is NOT a component
210-
const utilFile = normalizePath('/workspace/src/utils.ts')
261+
const utilFile = normalizePath(join(tempDir, 'src', 'utils.ts'))
211262
const mockModules = [{ id: utilFile }]
212263
const ctx = createMockHmrContext(utilFile, mockModules)
213264

napi/angular-compiler/vite-plugin/index.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -377,14 +377,6 @@ export function angular(options: PluginOptions = {}): Plugin[] {
377377
if (mod) {
378378
server.moduleGraph.invalidateModule(mod)
379379
}
380-
381-
// Emit a synthetic change event on Vite's watcher so that other plugins
382-
// (e.g., @tailwindcss/vite, PostCSS) are notified that this content file
383-
// changed. Without this, tools like Tailwind won't rescan for new utility
384-
// classes added in template files, since we unwatched them from Vite.
385-
// Our handleHotUpdate still returns [] for component resources, preventing
386-
// Vite from triggering a full page reload.
387-
server.watcher.emit('change', file)
388380
}
389381
}
390382
}
@@ -660,7 +652,9 @@ export function angular(options: PluginOptions = {}): Plugin[] {
660652
if (/\.(html?|css|scss|sass|less)$/.test(ctx.file)) {
661653
const normalizedFile = normalizePath(ctx.file)
662654
if (resourceToComponent.has(normalizedFile)) {
663-
debugHmr('ignoring component resource file in handleHotUpdate (handled by custom watcher)')
655+
debugHmr(
656+
'ignoring component resource file in handleHotUpdate (handled by custom watcher)',
657+
)
664658
return []
665659
}
666660
debugHmr('letting non-component resource file through to Vite HMR: %s', normalizedFile)

0 commit comments

Comments
 (0)