From 3261b8cb13ded824cd3f590fbdba4c69f19fbf59 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 17:33:49 +0800 Subject: [PATCH 01/10] fix(vite): resolve @ng/component styles per class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HMR endpoint built the update module for any requested class from the FILE's styleUrls, which extractComponentUrls returns as the union of every component in the file. In a multi-component file a class was served its siblings' stylesheets. The same block held a second defect of the shape #449 fixed on the template side: the file-level list was tried FIRST, so being non-empty it shadowed the inline `styles:` of a class that declares no external ones. One root cause, both fixed here. Resolve per class, in the order the template block above already uses: the class's own styleUrls, then its inline styles, then the file-level list as a fallback for decorator shapes the locator cannot parse. Both Angular spellings are handled — `styleUrls: [...]` and the singular `styleUrl: '...'` — with the cross-match guards under test: neither url field matches the other, and neither matches inline `styles:`. Note for future tests in hmr-hot-update.test.ts: the shared setupPluginWithServer passes a stub resolved config, so preprocessCSS throws and the per-style catch drops every external stylesheet. The new endpoint tests build a real config via resolveConfig; without it they could not observe external styles at all. Fixes #451 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 60 ++++++ .../test/hmr-hot-update.test.ts | 186 +++++++++++++++++- napi/angular-compiler/vite-plugin/index.ts | 65 +++++- .../vite-plugin/utils/decorator-fields.ts | 47 +++++ 4 files changed, 350 insertions(+), 8 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index efa0ba655..ee93fe191 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -3,6 +3,8 @@ import { describe, expect, it } from 'vitest' import { emptyDelimitedRange, locateComponentDecorators, + locateStyleUrlFor, + locateStyleUrlsFor, locateStylesFieldFor, locateTemplateStringFor, locateTemplateUrlFor, @@ -304,6 +306,64 @@ describe('decorator-fields utils', () => { }) }) + describe('locateStyleUrlsFor / locateStyleUrlFor', () => { + const multi = ` + @Component({ selector: 'a', styleUrls: ['./first.css'] }) + export class FirstComponent {} + @Component({ selector: 'b', styleUrls: ['./second.css', './extra.css'] }) + export class SecondComponent {} + ` + + it('returns null when className matches no decorator', () => { + expect(locateStyleUrlsFor(multi, 'Nope')).toBeNull() + expect(locateStyleUrlFor(multi, 'Nope')).toBeNull() + }) + + it('returns each component its own styleUrls range in a multi-component file', () => { + const first = locateStyleUrlsFor(multi, 'FirstComponent')! + const second = locateStyleUrlsFor(multi, 'SecondComponent')! + expect(multi.slice(first[0], first[1] + 1)).toBe(`['./first.css']`) + expect(multi.slice(second[0], second[1] + 1)).toBe(`['./second.css', './extra.css']`) + }) + + it('locates the singular `styleUrl:` string form', () => { + const src = `@Component({ styleUrl: './solo.css' })\nexport class Foo {}` + const range = locateStyleUrlFor(src, 'Foo')! + expect(src.slice(range[0], range[1] + 1)).toBe(`'./solo.css'`) + }) + + // The four cross-match guards: `styleUrl`, `styleUrls` and `styles` are + // distinct fields and must never resolve to one another. + it('does not match `styleUrls:` when looking for the singular `styleUrl:`', () => { + const src = `@Component({ styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleUrlFor(src, 'Foo')).toBeNull() + }) + + it('does not match the singular `styleUrl:` when looking for `styleUrls:`', () => { + const src = `@Component({ styleUrl: './a.css' })\nexport class Foo {}` + expect(locateStyleUrlsFor(src, 'Foo')).toBeNull() + }) + + it('does not match an inline `styles:` field as either url field', () => { + const src = `@Component({ styles: ['.a { color: red }'] })\nexport class Foo {}` + expect(locateStyleUrlsFor(src, 'Foo')).toBeNull() + expect(locateStyleUrlFor(src, 'Foo')).toBeNull() + }) + + it('does not match either url field as the inline `styles:` field', () => { + const urls = `@Component({ styleUrls: ['./a.css'] })\nexport class Foo {}` + const url = `@Component({ styleUrl: './a.css' })\nexport class Foo {}` + expect(locateStylesFieldFor(urls, 'Foo')).toBeNull() + expect(locateStylesFieldFor(url, 'Foo')).toBeNull() + }) + + it('finds styleUrls when the decorator also has an inline styles field', () => { + const src = `@Component({ styles: ['.x{}'], styleUrls: ['./real.css'] })\nexport class Foo {}` + const range = locateStyleUrlsFor(src, 'Foo')! + expect(src.slice(range[0], range[1] + 1)).toBe(`['./real.css']`) + }) + }) + // ----------------------------------------------------------------- // Comment-aware scanning. Without this, the walker treats `'` in a // `// don't ...` line comment as opening a string literal that never diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index cc57a00d0..b408ec27a 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -16,7 +16,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import type { Plugin, ModuleNode, HmrContext } from 'vite' -import { normalizePath } from 'vite' +import { normalizePath, resolveConfig } from 'vite' import { afterAll, beforeAll, describe, it, expect, vi } from 'vitest' import { angular } from '../vite-plugin/index.js' @@ -1335,3 +1335,187 @@ describe('@ng/component endpoint resolves the template per class', () => { expect(body).not.toContain('PC_EXT_MARKER') }) }) + +describe('@ng/component endpoint resolves the styles per class', () => { + // preprocessCSS needs a REAL resolved config to run; the shared mock config + // makes every external stylesheet fail to preprocess, which would hide what + // these tests are about. Other tests in this file keep the mock. + async function setupPluginWithRealConfig(plugin: Plugin) { + const mockServer = createMockServer() + + await callPluginHook( + plugin.config as Plugin['config'], + {} as any, + { command: 'serve', mode: 'development' } as any, + ) + const resolved = await resolveConfig( + { configFile: false, root: tempDir, logLevel: 'silent' }, + 'serve', + ) + await callPluginHook(plugin.configResolved as Plugin['configResolved'], resolved as any) + + if (typeof plugin.configureServer === 'function') { + await (plugin.configureServer as Function)(mockServer) + } + ;(mockServer as any).__angularWatchTemplate = () => {} + + return mockServer + } + + async function transformSource(plugin: Plugin, source: string, path: string) { + if (!plugin.transform || typeof plugin.transform === 'function') { + throw new Error('Expected plugin transform handler') + } + await plugin.transform.handler.call( + { error() {}, warn() {}, addWatchFile() {} } as any, + source, + path, + ) + } + + function getMiddleware(mockServer: any) { + const middleware = (mockServer.middlewares.use as ReturnType).mock.calls[0]?.[0] + expect(middleware, 'expected middleware to be registered').toBeDefined() + return middleware + } + + it('serves each component its own styleUrls in a multi-component file', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const firstCssPath = join(appDir, 'ps-first.component.css') + const secondCssPath = join(appDir, 'ps-second.component.css') + const multiPath = join(appDir, 'ps-multi.component.ts') + writeFileSync(firstCssPath, '.PS_FIRST_MARKER { color: red; }') + writeFileSync(secondCssPath, '.PS_SECOND_MARKER { color: blue; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-first', + template: '

first

', + styleUrls: ['./ps-first.component.css'], + }) + export class FirstComponent {} + @Component({ + selector: 'app-ps-second', + template: '

second

', + styleUrls: ['./ps-second.component.css'], + }) + export class SecondComponent {} + ` + writeFileSync(multiPath, source) + await transformSource(plugin, source, multiPath) + + // Edit the SECOND component's stylesheet; the fan-out queues both classes. + writeFileSync(secondCssPath, '.PS_SECOND_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(secondCssPath), + [{ id: normalizePath(secondCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + // SecondComponent's update module must carry only its OWN stylesheet, not + // the union of every styleUrl declared in the file. + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${multiPath}@SecondComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_SECOND_MARKER') + expect(body).not.toContain('PS_FIRST_MARKER') + }) + + it('serves the singular `styleUrl` of the requested class', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const soloCssPath = join(appDir, 'ps-solo.component.css') + const otherCssPath = join(appDir, 'ps-other.component.css') + const singularPath = join(appDir, 'ps-singular.component.ts') + writeFileSync(soloCssPath, '.PS_SOLO_MARKER { color: red; }') + writeFileSync(otherCssPath, '.PS_OTHER_MARKER { color: blue; }') + + // `styleUrl` (singular, Angular 17+) is a bare string, not an array. + // It is declared SECOND here so the file-level list does not happen to + // start with it. + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-other', + template: '

other

', + styleUrl: './ps-other.component.css', + }) + export class OtherComponent {} + @Component({ + selector: 'app-ps-solo', + template: '

solo

', + styleUrl: './ps-solo.component.css', + }) + export class SoloComponent {} + ` + writeFileSync(singularPath, source) + await transformSource(plugin, source, singularPath) + + writeFileSync(soloCssPath, '.PS_SOLO_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(soloCssPath), + [{ id: normalizePath(soloCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${singularPath}@SoloComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_SOLO_MARKER') + expect(body).not.toContain('PS_OTHER_MARKER') + }) + + it('serves the inline styles of a class whose sibling uses a styleUrl', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const extCssPath = join(appDir, 'ps-ext.component.css') + const mixedPath = join(appDir, 'ps-mixed.component.ts') + writeFileSync(extCssPath, '.PS_EXT_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-ext', + template: '

ext

', + styleUrls: ['./ps-ext.component.css'], + }) + export class ExtComponent {} + @Component({ + selector: 'app-ps-inline', + template: '

inline

', + styles: ['.PS_INLINE_MARKER { color: blue; }'], + }) + export class InlineComponent {} + ` + writeFileSync(mixedPath, source) + await transformSource(plugin, source, mixedPath) + + // Edit only the inline styles; the strip-equality branch queues both + // classes in the file. + const edited = source.replace('color: blue', 'color: green') + writeFileSync(mixedPath, edited) + const ctx = createMockHmrContext(mixedPath, [{ id: mixedPath }], mockServer) + await callHandleHotUpdate(plugin, ctx) + + // InlineComponent must get its OWN inline styles — a sibling's styleUrl + // making the FILE-level list non-empty must not shadow the inline branch. + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${mixedPath}@InlineComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_INLINE_MARKER') + expect(body).not.toContain('PS_EXT_MARKER') + }) +}) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index 62a594337..8d3147ba5 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -39,6 +39,8 @@ import { emptyDelimitedRange, locateComponentDecorators, locateStylesFieldFor, + locateStyleUrlFor, + locateStyleUrlsFor, locateStylesInArgs, locateTemplateInArgs, locateTemplateStringFor, @@ -686,10 +688,18 @@ export function angular(options: PluginOptions = {}): Plugin[] { // disk and run through Vite's preprocessCSS (so SCSS/LESS // resolve correctly); inline styles are extracted from the // .ts source as plain CSS strings. - let styles: string[] | null = null - if (styleUrls.length > 0) { + // + // Resolve per CLASS, like the template above: the file-level + // `styleUrls` is the union of every component in the file, so + // in a multi-component file it served a class its siblings' + // stylesheets — and, being non-empty, it also shadowed the + // inline `styles:` of a class that has no external ones. + // Fall back to the file-level list only for decorator shapes + // the per-class locator cannot parse (preserves the old + // behavior there). + const readStyles = async (urls: string[]): Promise => { const styleContents: string[] = [] - for (const styleUrl of styleUrls) { + for (const styleUrl of urls) { const stylePath = resolve(dir, styleUrl) try { let styleContent = await readFile(stylePath, 'utf-8') @@ -706,14 +716,18 @@ export function angular(options: PluginOptions = {}): Plugin[] { // Style file not found, continue without this style } } - if (styleContents.length > 0) { - styles = styleContents - } + return styleContents.length > 0 ? styleContents : null + } + let styles: string[] | null = null + const classStyleUrls = extractStyleUrlsFor(source, className) + if (classStyleUrls !== null) { + styles = await readStyles(classStyleUrls) } else { - // No external styleUrls — fall back to inline `styles: […]`. const inlineStyles = extractInlineStyles(source, className) if (inlineStyles !== null && inlineStyles.length > 0) { styles = inlineStyles + } else if (styleUrls.length > 0) { + styles = await readStyles(styleUrls) } } @@ -1428,6 +1442,43 @@ function extractTemplateUrlFor(code: string, className: string): string | null { return code.slice(range[0] + 1, range[1]) } +/** + * Extract the external style URLs declared by the `@Component({...})` + * decorator that decorates the class named `className`. + * + * Handles both Angular spellings: + * - `styleUrls: ['./a.css', './b.css']` → each literal, in order. + * - `styleUrl: './a.css'` (17+) → a one-element array. + * + * `styleUrls` wins if a decorator somehow carries both; Angular itself + * rejects that combination, so this only makes the choice deterministic. + * + * Returns null if the named decorator declares neither field, or its value + * is something other than a string/array literal (e.g. a variable). + */ +function extractStyleUrlsFor(code: string, className: string): string[] | null { + const pluralRange = locateStyleUrlsFor(code, className) + if (pluralRange) { + const opener = code[pluralRange[0]] + if (opener !== '[') { + // Bare string in the plural field — lenient, treat as one entry. + return [code.slice(pluralRange[0] + 1, pluralRange[1])] + } + // Array form — walk string literals inside the array body in order. + const body = code.slice(pluralRange[0] + 1, pluralRange[1]) + const stringRe = /`([\s\S]*?)`|'((?:\\.|[^'\\])*)'|"((?:\\.|[^"\\])*)"/g + const urls: string[] = [] + let m: RegExpExecArray | null + while ((m = stringRe.exec(body)) !== null) { + urls.push(m[1] ?? m[2] ?? m[3] ?? '') + } + return urls.length > 0 ? urls : null + } + const singularRange = locateStyleUrlFor(code, className) + if (!singularRange) return null + return [code.slice(singularRange[0] + 1, singularRange[1])] +} + /** * Extract the inline styles from the `@Component({...})` decorator that * decorates the class named `className`, as a positional array. diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index 2dbc51448..5a47e397b 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -435,3 +435,50 @@ export function locateTemplateUrlFor(code: string, className: string): [number, const found = locateComponentDecorators(code).find((d) => d.className === className) return found ? locateTemplateUrlInArgs(code, found.argsRange) : null } + +/** + * Locate the `styleUrls:` field inside a specific `@Component(...)` decorator + * identified by its `argsRange`. See `locateStylesInArgs` for when to prefer + * this over the className-based variant. Field matching is word-bounded, so + * `styleUrls` never matches the singular `styleUrl:` or the inline `styles:`. + */ +export function locateStyleUrlsInArgs( + code: string, + argsRange: [number, number], +): [number, number] | null { + return locateFieldInsideArgs(code, argsRange, 'styleUrls', STYLES_OPENERS) +} + +/** + * Locate the singular `styleUrl:` string field (Angular 17+) inside a + * specific `@Component(...)` decorator identified by its `argsRange`. Its + * value is a bare string, never an array — see `locateStyleUrlsInArgs` for + * the plural form. + */ +export function locateStyleUrlInArgs( + code: string, + argsRange: [number, number], +): [number, number] | null { + return locateFieldInsideArgs(code, argsRange, 'styleUrl', TEMPLATE_OPENERS) +} + +/** + * Locate the `styleUrls:` field inside the `@Component(...)` decorator that + * decorates the class named `className`. Convenience wrapper that finds the + * decorator by className and delegates to `locateStyleUrlsInArgs`. + */ +export function locateStyleUrlsFor(code: string, className: string): [number, number] | null { + const found = locateComponentDecorators(code).find((d) => d.className === className) + return found ? locateStyleUrlsInArgs(code, found.argsRange) : null +} + +/** + * Locate the singular `styleUrl:` field inside the `@Component(...)` + * decorator that decorates the class named `className`. Convenience wrapper + * that finds the decorator by className and delegates to + * `locateStyleUrlInArgs`. + */ +export function locateStyleUrlFor(code: string, className: string): [number, number] | null { + const found = locateComponentDecorators(code).find((d) => d.className === className) + return found ? locateStyleUrlInArgs(code, found.argsRange) : null +} From c853252c89816df30e743096bbbf9d8ebf751e48 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 17:47:43 +0800 Subject: [PATCH 02/10] fix(vite): parse decorator style arrays, and serve each class exactly its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things, all in the endpoint's style resolution. 1. Replace the regex string-walking in extractStyleUrlsFor and its twin in extractInlineStyles with a real scanner, readStringLiterals, built on the file's existing skipComment and findClosingDelim. The regex was blind to comments: `['/* don't */ './a.css']` yielded ["t touch */ "] and dropped the real URL. That is a main bug on the inline-styles path, which has shipped with the regex. A 16-input differential probe shows the two disagree on exactly the 3 comment cases and nowhere else. 2. A class declaring no styles at all fell through to the file-level union and was served a sibling's scoped CSS — the isolation defect this change exists to remove. 3. A class declaring both `styles` and `styleUrls` lost the inline half; the branches were exclusive. The compiler merges them, so HMR diverged from compile after any update. The fix distinguishes three states, which the old null could not express: the decorator is not locatable (fall back to the file-level list), it is locatable with no style field (serve nothing — a real empty answer), or it declares a field holding no string literal (fall back, since the Rust extractor folds constants this text scan cannot). Merge order is inline first, resolved external appended, taken from the implementation rather than assumed: decorator.rs assigns the inline array into metadata.styles and transform.rs pushes resolved content onto it. The contract test asserts presence only, so it would not have caught a reversal. Whitespace-only entries are dropped, matching Angular's trim filter. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 110 ++++++++++ .../test/hmr-hot-update.test.ts | 203 ++++++++++++++++++ napi/angular-compiler/vite-plugin/index.ts | 115 +++++----- .../vite-plugin/utils/decorator-fields.ts | 92 ++++++++ 4 files changed, 460 insertions(+), 60 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index ee93fe191..b7fcb720b 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -3,11 +3,13 @@ import { describe, expect, it } from 'vitest' import { emptyDelimitedRange, locateComponentDecorators, + locateStyleFieldsFor, locateStyleUrlFor, locateStyleUrlsFor, locateStylesFieldFor, locateTemplateStringFor, locateTemplateUrlFor, + readStringLiterals, } from '../vite-plugin/utils/decorator-fields.js' describe('decorator-fields utils', () => { @@ -364,6 +366,54 @@ describe('decorator-fields utils', () => { }) }) + // The endpoint needs three answers, not two: a class with no styles must be + // served none, while a class whose styles cannot be read must fall back to + // the file-level list. Only a null return distinguishes them. + describe('locateStyleFieldsFor', () => { + it('returns null when className matches no decorator', () => { + const src = `@Component({ styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Nope')).toBeNull() + }) + + it('returns both members null for a class that declares no styles', () => { + const src = `@Component({ template: '

' })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ urls: null, inline: null }) + }) + + it('reports a field that is present but holds no literal', () => { + const src = `@Component({ styleUrls: [STYLE_URL] })\nexport class Foo {}` + const fields = locateStyleFieldsFor(src, 'Foo')! + expect(fields.urls).not.toBeNull() + expect(readStringLiterals(src, fields.urls!)).toEqual([]) + }) + + it('reports inline and url ranges together when a class declares both', () => { + const src = `@Component({ styles: ['.x{}'], styleUrls: ['./real.css'] })\nexport class Foo {}` + const fields = locateStyleFieldsFor(src, 'Foo')! + expect(readStringLiterals(src, fields.inline!)).toEqual(['.x{}']) + expect(readStringLiterals(src, fields.urls!)).toEqual(['./real.css']) + }) + + it('falls back to the singular `styleUrl` for the urls member', () => { + const src = `@Component({ styleUrl: './solo.css' })\nexport class Foo {}` + const fields = locateStyleFieldsFor(src, 'Foo')! + expect(readStringLiterals(src, fields.urls!)).toEqual(['./solo.css']) + expect(fields.inline).toBeNull() + }) + + it('resolves each class separately in a multi-component file', () => { + const src = ` + @Component({ selector: 'a', styleUrls: ['./a.css'] }) + export class StyledComponent {} + @Component({ selector: 'b', template: '

' }) + export class BareComponent {} + ` + const styled = locateStyleFieldsFor(src, 'StyledComponent')! + expect(readStringLiterals(src, styled.urls!)).toEqual(['./a.css']) + expect(locateStyleFieldsFor(src, 'BareComponent')).toEqual({ urls: null, inline: null }) + }) + }) + // ----------------------------------------------------------------- // Comment-aware scanning. Without this, the walker treats `'` in a // `// don't ...` line comment as opening a string literal that never @@ -371,6 +421,66 @@ describe('decorator-fields utils', () => { // or `/* styles: [...] */` block comment as a real field (wrong // range returned). // ----------------------------------------------------------------- + describe('readStringLiterals', () => { + // Read the value of `styles:` from a one-component source, so these + // exercise the same path the extractors use. + const literalsOf = (value: string): string[] => { + const src = `@Component({ styles: ${value} })\nclass Foo {}` + return readStringLiterals(src, locateStylesFieldFor(src, 'Foo')!) + } + + it('reads every entry of an array in order', () => { + expect(literalsOf(`['./a.css', './b.css']`)).toEqual(['./a.css', './b.css']) + }) + + it('reads a single-entry array', () => { + expect(literalsOf(`['./a.css']`)).toEqual(['./a.css']) + }) + + it('reads a bare string value as one entry', () => { + expect(literalsOf(`'./a.css'`)).toEqual(['./a.css']) + }) + + it('reads mixed quote styles, including template literals', () => { + expect(literalsOf('[\'./a.css\', "./b.css", `./c.css`]')).toEqual([ + './a.css', + './b.css', + './c.css', + ]) + }) + + it('returns no entries for an empty array', () => { + expect(literalsOf(`[]`)).toEqual([]) + }) + + it('keeps an escaped quote inside a literal, unescaped', () => { + expect(literalsOf(`['it\\'s.css']`)).toEqual([`it\\'s.css`]) + }) + + it('ignores an apostrophe inside a block comment before an entry', () => { + expect(literalsOf(`[/* don't drop this */ './a.css']`)).toEqual(['./a.css']) + }) + + it('ignores an apostrophe inside a line comment before an entry', () => { + expect(literalsOf(`[\n // it's here\n './a.css',\n]`)).toEqual(['./a.css']) + }) + + it('ignores a comment between two entries', () => { + expect(literalsOf(`['./a.css', /* don't */ './b.css']`)).toEqual(['./a.css', './b.css']) + }) + + it('skips a non-literal entry rather than failing', () => { + expect(literalsOf(`[SOME_CONST, './a.css']`)).toEqual(['./a.css']) + }) + + it('stops at an unterminated literal and returns what it read', () => { + // The locator bounds the range, so the unterminated entry is dropped. + const src = `@Component({ styles: ['./a.css', './b.css] })\nclass Foo {}` + const range = locateStylesFieldFor(src, 'Foo') + if (range) expect(readStringLiterals(src, range)).toEqual(['./a.css']) + }) + }) + describe('comment handling in @Component args', () => { it('does not get stuck on an apostrophe inside a line comment', () => { const src = `@Component({ diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index b408ec27a..416637183 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -1518,4 +1518,207 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).toContain('PS_INLINE_MARKER') expect(body).not.toContain('PS_EXT_MARKER') }) + it('serves the styleUrls of a class whose array carries a comment', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const realCssPath = join(appDir, 'ps-commented.component.css') + const commentedPath = join(appDir, 'ps-commented.component.ts') + writeFileSync(realCssPath, '.PS_COMMENTED_MARKER { color: red; }') + + // An apostrophe inside a comment in the array body must not be mistaken + // for a string delimiter, which would swallow the real entry. + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-commented', + template: '

commented

', + styleUrls: [ + /* don't drop this */ + './ps-commented.component.css', + ], + }) + export class CommentedComponent {} + ` + writeFileSync(commentedPath, source) + await transformSource(plugin, source, commentedPath) + + writeFileSync(realCssPath, '.PS_COMMENTED_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(realCssPath), + [{ id: normalizePath(realCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${commentedPath}@CommentedComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_COMMENTED_MARKER') + }) + + it('serves the inline styles of a class whose array carries a comment', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const inlineCommentPath = join(appDir, 'ps-inline-comment.component.ts') + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-inline-comment', + template: '

inline

', + styles: [ + /* it's fine */ + '.PS_INLINE_COMMENT_MARKER { color: red; }', + ], + }) + export class InlineCommentComponent {} + ` + writeFileSync(inlineCommentPath, source) + await transformSource(plugin, source, inlineCommentPath) + + const edited = source.replace('color: red', 'color: green') + writeFileSync(inlineCommentPath, edited) + const ctx = createMockHmrContext( + normalizePath(inlineCommentPath), + [{ id: normalizePath(inlineCommentPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${inlineCommentPath}@InlineCommentComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_INLINE_COMMENT_MARKER') + }) + it('serves no styles to a class that declares none, even beside a styled sibling', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const styledCssPath = join(appDir, 'ps-bare-styled.component.css') + const barePath = join(appDir, 'ps-bare.component.ts') + writeFileSync(styledCssPath, '.PS_BARE_SIBLING_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-styled', + template: '

styled

', + styleUrls: ['./ps-bare-styled.component.css'], + }) + export class StyledComponent {} + @Component({ + selector: 'app-ps-bare', + template: '

bare

', + }) + export class BareComponent {} + ` + writeFileSync(barePath, source) + await transformSource(plugin, source, barePath) + + writeFileSync(styledCssPath, '.PS_BARE_SIBLING_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(styledCssPath), + [{ id: normalizePath(styledCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + // BareComponent declares no styles at all: it must get NONE, not the + // file-level union of its sibling's scoped CSS. + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${barePath}@BareComponent`, + ) + expect(body).not.toBe('') + expect(body).not.toContain('PS_BARE_SIBLING_MARKER') + expect(body).not.toContain('styles:') + }) + + it('serves both inline styles and styleUrls when a class declares both', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const extCssPath = join(appDir, 'ps-merge-ext.component.css') + const mergePath = join(appDir, 'ps-merge.component.ts') + writeFileSync(extCssPath, '.PS_MERGE_EXTERNAL_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-merge', + template: '

merge

', + styles: ['.PS_MERGE_INLINE_MARKER { color: blue; }'], + styleUrls: ['./ps-merge-ext.component.css'], + }) + export class MergeComponent {} + ` + writeFileSync(mergePath, source) + await transformSource(plugin, source, mergePath) + + writeFileSync(extCssPath, '.PS_MERGE_EXTERNAL_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(extCssPath), + [{ id: normalizePath(extCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${mergePath}@MergeComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_MERGE_INLINE_MARKER') + expect(body).toContain('PS_MERGE_EXTERNAL_MARKER') + // Angular's order: the decorator's own inline `styles` first, then the + // resolved `styleUrl(s)` content appended (see `resolve_styles`). + expect(body.indexOf('PS_MERGE_INLINE_MARKER')).toBeLessThan( + body.indexOf('PS_MERGE_EXTERNAL_MARKER'), + ) + }) + + it('falls back to the file-level styleUrls when a class uses a non-literal entry', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const constCssPath = join(appDir, 'ps-const.component.css') + const constPath = join(appDir, 'ps-const.component.ts') + writeFileSync(constCssPath, '.PS_CONST_MARKER { color: red; }') + + // The URL comes from a const, so the per-class locator finds the field but + // reads no string literal out of it. That must fall back to the file-level + // list (which the Rust extractor DOES fold), not serve an empty style set. + const source = ` + import { Component } from '@angular/core'; + const STYLE_URL = './ps-const.component.css'; + @Component({ + selector: 'app-ps-const', + template: '

const

', + styleUrls: [STYLE_URL], + }) + export class ConstComponent {} + ` + writeFileSync(constPath, source) + await transformSource(plugin, source, constPath) + + writeFileSync(constCssPath, '.PS_CONST_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(constCssPath), + [{ id: normalizePath(constCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${constPath}@ConstComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_CONST_MARKER') + }) }) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index 8d3147ba5..7e93f5465 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -39,12 +39,12 @@ import { emptyDelimitedRange, locateComponentDecorators, locateStylesFieldFor, - locateStyleUrlFor, - locateStyleUrlsFor, + locateStyleFieldsFor, locateStylesInArgs, locateTemplateInArgs, locateTemplateStringFor, locateTemplateUrlFor, + readStringLiterals, } from './utils/decorator-fields.js' import { injectDtsDeclarations } from './utils/dts.js' @@ -692,11 +692,11 @@ export function angular(options: PluginOptions = {}): Plugin[] { // Resolve per CLASS, like the template above: the file-level // `styleUrls` is the union of every component in the file, so // in a multi-component file it served a class its siblings' - // stylesheets — and, being non-empty, it also shadowed the - // inline `styles:` of a class that has no external ones. - // Fall back to the file-level list only for decorator shapes - // the per-class locator cannot parse (preserves the old - // behavior there). + // stylesheets — including a class declaring no styles at all — + // and, being non-empty, it also shadowed the inline `styles:` + // of a class that has no external ones. Fall back to the + // file-level list only when the class's own styles cannot be + // read (preserves the old behavior there). const readStyles = async (urls: string[]): Promise => { const styleContents: string[] = [] for (const styleUrl of urls) { @@ -719,16 +719,25 @@ export function angular(options: PluginOptions = {}): Plugin[] { return styleContents.length > 0 ? styleContents : null } let styles: string[] | null = null - const classStyleUrls = extractStyleUrlsFor(source, className) - if (classStyleUrls !== null) { - styles = await readStyles(classStyleUrls) - } else { - const inlineStyles = extractInlineStyles(source, className) - if (inlineStyles !== null && inlineStyles.length > 0) { - styles = inlineStyles - } else if (styleUrls.length > 0) { - styles = await readStyles(styleUrls) - } + const classStyles = extractClassStylesFor(source, className) + if (classStyles !== null) { + // Inline `styles` first, then the resolved `styleUrl(s)` + // content appended — the order `resolve_styles` produces in + // the compiler, which pushes resolved content onto the + // decorator's own `styles`. Whitespace-only entries are + // dropped to match Angular's `style.trim().length > 0`. + const external = + classStyles.urls.length > 0 ? ((await readStyles(classStyles.urls)) ?? []) : [] + const merged = [...classStyles.inline, ...external].filter( + (style) => style.trim().length > 0, + ) + styles = merged.length > 0 ? merged : null + } else if (styleUrls.length > 0) { + // The class's own styles could not be read — a decorator + // shape this text scan cannot parse, e.g. a styleUrl built + // from a constant the Rust extractor folds. Fall back to the + // file-level list, preserving the old behavior there. + styles = await readStyles(styleUrls) } const result = compileForHmrSync(templateContent, className, resolvedId, styles, { @@ -1443,40 +1452,38 @@ function extractTemplateUrlFor(code: string, className: string): string | null { } /** - * Extract the external style URLs declared by the `@Component({...})` - * decorator that decorates the class named `className`. + * The styles one class declares in its own `@Component({...})`, split by + * source: inline `styles` and external `styleUrl(s)`. * - * Handles both Angular spellings: - * - `styleUrls: ['./a.css', './b.css']` → each literal, in order. - * - `styleUrl: './a.css'` (17+) → a one-element array. + * Handles every shape Angular accepts — `styles` and `styleUrls` as an array + * or a bare literal, plus the singular `styleUrl` (17+). Both arrays are + * empty for a class that declares no styles at all, which is a real answer: + * that component must be served nothing. * - * `styleUrls` wins if a decorator somehow carries both; Angular itself - * rejects that combination, so this only makes the choice deterministic. - * - * Returns null if the named decorator declares neither field, or its value - * is something other than a string/array literal (e.g. a variable). + * Returns null when the answer is *unknown* rather than empty — the class's + * decorator could not be located, or it declares a style field whose value + * holds no string literal (a constant, an identifier, an unresolved + * interpolation). The caller falls back to the file-level list there, since + * the Rust extractor folds constants this text scan cannot. */ -function extractStyleUrlsFor(code: string, className: string): string[] | null { - const pluralRange = locateStyleUrlsFor(code, className) - if (pluralRange) { - const opener = code[pluralRange[0]] - if (opener !== '[') { - // Bare string in the plural field — lenient, treat as one entry. - return [code.slice(pluralRange[0] + 1, pluralRange[1])] - } - // Array form — walk string literals inside the array body in order. - const body = code.slice(pluralRange[0] + 1, pluralRange[1]) - const stringRe = /`([\s\S]*?)`|'((?:\\.|[^'\\])*)'|"((?:\\.|[^"\\])*)"/g - const urls: string[] = [] - let m: RegExpExecArray | null - while ((m = stringRe.exec(body)) !== null) { - urls.push(m[1] ?? m[2] ?? m[3] ?? '') - } - return urls.length > 0 ? urls : null +function extractClassStylesFor( + code: string, + className: string, +): { inline: string[]; urls: string[] } | null { + const fields = locateStyleFieldsFor(code, className) + if (!fields) return null + + let inline: string[] = [] + if (fields.inline) { + inline = readStringLiterals(code, fields.inline) + if (inline.length === 0) return null + } + let urls: string[] = [] + if (fields.urls) { + urls = readStringLiterals(code, fields.urls) + if (urls.length === 0) return null } - const singularRange = locateStyleUrlFor(code, className) - if (!singularRange) return null - return [code.slice(singularRange[0] + 1, singularRange[1])] + return { inline, urls } } /** @@ -1495,19 +1502,7 @@ function extractStyleUrlsFor(code: string, className: string): string[] | null { function extractInlineStyles(code: string, className: string): string[] | null { const range = locateStylesFieldFor(code, className) if (!range) return null - const opener = code[range[0]] - if (opener !== '[') { - // Bare string form — return the inner contents as a single element. - return [code.slice(range[0] + 1, range[1])] - } - // Array form — walk string literals inside the array body in order. - const body = code.slice(range[0] + 1, range[1]) - const stringRe = /`([\s\S]*?)`|'((?:\\.|[^'\\])*)'|"((?:\\.|[^"\\])*)"/g - const styles: string[] = [] - let m: RegExpExecArray | null - while ((m = stringRe.exec(body)) !== null) { - styles.push(m[1] ?? m[2] ?? m[3] ?? '') - } + const styles = readStringLiterals(code, range) return styles.length > 0 ? styles : null } diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index 5a47e397b..585ed84a0 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -482,3 +482,95 @@ export function locateStyleUrlFor(code: string, className: string): [number, num const found = locateComponentDecorators(code).find((d) => d.className === className) return found ? locateStyleUrlInArgs(code, found.argsRange) : null } + +/** + * Read the string literals out of a located field value, given the inclusive + * `[start, end]` range of its outer delimiters (the shape every `locate*` + * here returns). + * + * Handles both shapes Angular accepts for `styles` / `styleUrls`: + * - Array (`['…', "…", `…`]`, or any mix) → each literal, in order. + * - Bare single literal (`'…'`, `"…"`, `` `…` ``) → a one-element array. + * + * Inside the array body, whitespace, commas and comments are skipped, and + * each literal is delimited with `findClosingDelim`, so escape sequences and + * apostrophes inside comments cannot be mistaken for delimiters. Anything + * that is not a string literal (an identifier, a spread, a nested array) is + * skipped rather than treated as an entry. + * + * Returns the raw inner contents — no unescaping, no trimming — because HMR + * delivers these verbatim. An unterminated literal ends the scan, returning + * whatever was collected before it. + */ +export function readStringLiterals(code: string, range: [number, number]): string[] { + const [start, end] = range + if (code[start] !== '[') { + // Bare literal — the range already delimits it. + return [code.slice(start + 1, end)] + } + + const literals: string[] = [] + let i = start + 1 + while (i < end) { + const ch = code[i] + if (WS_RE.test(ch) || ch === ',') { + i++ + continue + } + const afterComment = skipComment(code, i, end) + if (afterComment !== -1) { + i = afterComment + continue + } + if (ch === "'" || ch === '"' || ch === '`') { + const close = findClosingDelim(code, i) + // Unterminated literal: nothing further can be read reliably. + if (close === -1 || close >= end) break + literals.push(code.slice(i + 1, close)) + i = close + 1 + continue + } + // Not a literal (identifier, spread, nested array…) — skip this + // character. Every branch above advances `i`, so the scan terminates. + i++ + } + return literals +} + +/** + * The value ranges of the style-related fields on one `@Component(...)`. + * A null member means the decorator does not declare that field at all — + * which is different from declaring it with a value no literal can be read + * from (a constant, an identifier), where the range is present but + * `readStringLiterals` returns nothing. + */ +export interface ClassStyleFieldRanges { + /** `styleUrls: [...]`, else the singular `styleUrl: '...'`. */ + urls: [number, number] | null + /** Inline `styles: [...] | '...'`. */ + inline: [number, number] | null +} + +/** + * Locate the style fields of the `@Component(...)` decorating `className`. + * + * Returns null when no such decorator could be located, so a caller can tell + * "this class declares no styles" (both members null) from "this class could + * not be read" — the two need opposite fallbacks. + * + * `styleUrls` wins over `styleUrl` if a decorator somehow carries both; + * Angular itself rejects that combination, so this only makes the choice + * deterministic. One decorator scan serves all three lookups. + */ +export function locateStyleFieldsFor( + code: string, + className: string, +): ClassStyleFieldRanges | null { + const found = locateComponentDecorators(code).find((d) => d.className === className) + if (!found) return null + return { + urls: + locateStyleUrlsInArgs(code, found.argsRange) ?? locateStyleUrlInArgs(code, found.argsRange), + inline: locateStylesInArgs(code, found.argsRange), + } +} From 9066e4325b6fa4e2433252bcdc76599448b5108a Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 18:08:06 +0800 Subject: [PATCH 03/10] fix(vite): classify a style field by presence, not by literal count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three-way classifier inferred completeness from how many literals it read, which got two cases backwards. - `styleUrl: STYLE_URL` yields no locator range, because an identifier is not a quote opener. That was read as "field absent", i.e. a confident "this class has no styles", so the endpoint served none — stripping the CSS of a component whose const the Rust extractor folds. Before this branch it fell back. Same for a mixed `[STYLE_URL, './b.css']`, which silently served only the literal half. - `styleUrls: []` is valid and means "no styles", but zero literals was read as unknown, so the fallback served a sibling's CSS. Field presence is now detected by the key, independent of the value, and the value is classified absent / literal / unreadable. An unreadable element anywhere makes the whole field unknown: acting on the literals beside it would drop whatever the rest names. An interpolated template literal counts as unreadable. Verified against the Rust extractor, which folds both consts and interpolations, so falling back is right. locateFieldInsideArgs keeps its nullable-range shape as a wrapper, so its existing callers are untouched — only the style path wants the third state. readStringLiterals now reports `complete` alongside its literals rather than gaining a sibling function that discards it. Also fix a Windows-only test failure caught by NAPI Smoke: one test passed the raw path to `transform` and a normalized one as `ctx.file`, which are the same file but different strings on Windows, so `componentsByFile.has(ctx.file)` missed and nothing was dispatched. Vite normalizes both in production, so this was a test artifact; the test now uses one spelling like its long-standing neighbours. An expectDispatched guard asserts the queued id matches the requested one, so this class of bug names itself instead of surfacing as an empty response body. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 111 ++++++-- .../test/hmr-hot-update.test.ts | 261 +++++++++++++++++- napi/angular-compiler/vite-plugin/index.ts | 23 +- .../vite-plugin/utils/decorator-fields.ts | 147 +++++++--- 4 files changed, 467 insertions(+), 75 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index b7fcb720b..c872cfa44 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -370,35 +370,57 @@ describe('decorator-fields utils', () => { // served none, while a class whose styles cannot be read must fall back to // the file-level list. Only a null return distinguishes them. describe('locateStyleFieldsFor', () => { + // Read the literals out of a located field, for the tests that care + // about the value rather than the classification. + const literalsIn = (src: string, field: { kind: string; range?: [number, number] }) => { + expect(field.kind).toBe('literal') + return readStringLiterals(src, (field as { range: [number, number] }).range).literals + } + it('returns null when className matches no decorator', () => { const src = `@Component({ styleUrls: ['./a.css'] })\nexport class Foo {}` expect(locateStyleFieldsFor(src, 'Nope')).toBeNull() }) - it('returns both members null for a class that declares no styles', () => { + it('reports both fields absent for a class that declares no styles', () => { const src = `@Component({ template: '

' })\nexport class Foo {}` - expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ urls: null, inline: null }) + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ + urls: { kind: 'absent' }, + inline: { kind: 'absent' }, + }) }) - it('reports a field that is present but holds no literal', () => { - const src = `@Component({ styleUrls: [STYLE_URL] })\nexport class Foo {}` - const fields = locateStyleFieldsFor(src, 'Foo')! - expect(fields.urls).not.toBeNull() - expect(readStringLiterals(src, fields.urls!)).toEqual([]) + it('reports a field whose value has no literal shape as unreadable', () => { + const src = `@Component({ styleUrls: STYLE_URLS })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) + }) + + it('reports a singular `styleUrl` identifier value as unreadable, not absent', () => { + // The regression this guards: reporting it absent tells the caller + // "this class has no styles", which strips the component's CSS. + const src = `@Component({ styleUrl: STYLE_URL })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) }) it('reports inline and url ranges together when a class declares both', () => { const src = `@Component({ styles: ['.x{}'], styleUrls: ['./real.css'] })\nexport class Foo {}` const fields = locateStyleFieldsFor(src, 'Foo')! - expect(readStringLiterals(src, fields.inline!)).toEqual(['.x{}']) - expect(readStringLiterals(src, fields.urls!)).toEqual(['./real.css']) + expect(literalsIn(src, fields.inline)).toEqual(['.x{}']) + expect(literalsIn(src, fields.urls)).toEqual(['./real.css']) }) it('falls back to the singular `styleUrl` for the urls member', () => { const src = `@Component({ styleUrl: './solo.css' })\nexport class Foo {}` const fields = locateStyleFieldsFor(src, 'Foo')! - expect(readStringLiterals(src, fields.urls!)).toEqual(['./solo.css']) - expect(fields.inline).toBeNull() + expect(literalsIn(src, fields.urls)).toEqual(['./solo.css']) + expect(fields.inline).toEqual({ kind: 'absent' }) + }) + + it('reports the urls member unreadable when only the singular form is unreadable', () => { + const src = `@Component({ styleUrl: STYLE_URL, styles: ['.x{}'] })\nexport class Foo {}` + const fields = locateStyleFieldsFor(src, 'Foo')! + expect(fields.urls).toEqual({ kind: 'unreadable' }) + expect(literalsIn(src, fields.inline)).toEqual(['.x{}']) }) it('resolves each class separately in a multi-component file', () => { @@ -409,8 +431,11 @@ describe('decorator-fields utils', () => { export class BareComponent {} ` const styled = locateStyleFieldsFor(src, 'StyledComponent')! - expect(readStringLiterals(src, styled.urls!)).toEqual(['./a.css']) - expect(locateStyleFieldsFor(src, 'BareComponent')).toEqual({ urls: null, inline: null }) + expect(literalsIn(src, styled.urls)).toEqual(['./a.css']) + expect(locateStyleFieldsFor(src, 'BareComponent')).toEqual({ + urls: { kind: 'absent' }, + inline: { kind: 'absent' }, + }) }) }) @@ -420,17 +445,24 @@ describe('decorator-fields utils', () => { // closes (real field missed), and a `// styles: [...]` line comment // or `/* styles: [...] */` block comment as a real field (wrong // range returned). + // + // `complete` reports whether every element was read. A caller acting + // on a partial list silently drops stylesheets. // ----------------------------------------------------------------- describe('readStringLiterals', () => { // Read the value of `styles:` from a one-component source, so these // exercise the same path the extractors use. - const literalsOf = (value: string): string[] => { + const readOf = (value: string) => { const src = `@Component({ styles: ${value} })\nclass Foo {}` return readStringLiterals(src, locateStylesFieldFor(src, 'Foo')!) } + const literalsOf = (value: string): string[] => readOf(value).literals it('reads every entry of an array in order', () => { - expect(literalsOf(`['./a.css', './b.css']`)).toEqual(['./a.css', './b.css']) + expect(readOf(`['./a.css', './b.css']`)).toEqual({ + literals: ['./a.css', './b.css'], + complete: true, + }) }) it('reads a single-entry array', () => { @@ -438,7 +470,7 @@ describe('decorator-fields utils', () => { }) it('reads a bare string value as one entry', () => { - expect(literalsOf(`'./a.css'`)).toEqual(['./a.css']) + expect(readOf(`'./a.css'`)).toEqual({ literals: ['./a.css'], complete: true }) }) it('reads mixed quote styles, including template literals', () => { @@ -449,8 +481,9 @@ describe('decorator-fields utils', () => { ]) }) - it('returns no entries for an empty array', () => { - expect(literalsOf(`[]`)).toEqual([]) + it('reports an empty array as complete, not unknown', () => { + // `styles: []` is a definitive answer: this class has no styles. + expect(readOf(`[]`)).toEqual({ literals: [], complete: true }) }) it('keeps an escaped quote inside a literal, unescaped', () => { @@ -469,15 +502,49 @@ describe('decorator-fields utils', () => { expect(literalsOf(`['./a.css', /* don't */ './b.css']`)).toEqual(['./a.css', './b.css']) }) - it('skips a non-literal entry rather than failing', () => { - expect(literalsOf(`[SOME_CONST, './a.css']`)).toEqual(['./a.css']) + it('reports a non-literal entry as incomplete', () => { + // The literal alongside it is not a partial answer to act on: acting + // on it would drop the stylesheet the constant resolves to. + expect(readOf(`[SOME_CONST, './a.css']`).complete).toBe(false) + }) + + it('reports a spread entry as incomplete', () => { + expect(readOf(`[...SHARED, './a.css']`).complete).toBe(false) + }) + + it('reports an interpolated template literal as incomplete', () => { + // The raw slice is `${DIR}/a.css`, not a real path. The Rust extractor + // folds it; this scan cannot. + expect(readOf('[`${DIR}/a.css`]').complete).toBe(false) + }) + + it('reports a bare interpolated template literal as incomplete', () => { + expect(readOf('`${DIR}/a.css`').complete).toBe(false) + }) + + it('treats an escaped `${` in a template literal as ordinary text', () => { + expect(readOf('[`\\${NOT_INTERPOLATED}.css`]')).toEqual({ + literals: ['\\${NOT_INTERPOLATED}.css'], + complete: true, + }) + }) + + it('treats `${` inside a quoted string as ordinary text', () => { + expect(readOf(`['\${DIR}/a.css']`)).toEqual({ + literals: ['${DIR}/a.css'], + complete: true, + }) }) - it('stops at an unterminated literal and returns what it read', () => { + it('stops at an unterminated literal and reports incomplete', () => { // The locator bounds the range, so the unterminated entry is dropped. const src = `@Component({ styles: ['./a.css', './b.css] })\nclass Foo {}` const range = locateStylesFieldFor(src, 'Foo') - if (range) expect(readStringLiterals(src, range)).toEqual(['./a.css']) + if (range) { + const read = readStringLiterals(src, range) + expect(read.literals).toEqual(['./a.css']) + expect(read.complete).toBe(false) + } }) }) diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index 416637183..dc6d94096 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -1204,6 +1204,20 @@ describe('@ng/component endpoint resolves the template per class', () => { ) } + // The component-file branch queues `pendingHmrUpdates` under `ctx.file` + // verbatim, and the endpoint looks the id up verbatim. Asserting the queued + // id here turns a spelling mismatch into a named failure instead of an + // unexplained empty body — the difference only shows up on Windows, where + // a normalized path and a `join()` path are different strings. + function expectDispatched(mockServer: any, componentId: string) { + const ids = mockServer._wsMessages + .filter((m: any) => m?.event === 'angular:component-update') + .map((m: any) => decodeURIComponent(m.data.id)) + expect(ids, 'expected the HMR dispatch to use the requested path spelling').toContain( + componentId, + ) + } + function getMiddleware(mockServer: any) { const middleware = (mockServer.middlewares.use as ReturnType).mock.calls[0]?.[0] expect(middleware, 'expected middleware to be registered').toBeDefined() @@ -1324,6 +1338,8 @@ describe('@ng/component endpoint resolves the template per class', () => { const ctx = createMockHmrContext(mixedPath, [{ id: mixedPath }], mockServer) await callHandleHotUpdate(plugin, ctx) + expectDispatched(mockServer, `${mixedPath}@InlineComponent`) + // InlineComponent must get its inline template — templateUrls.length > 0 // for the FILE must not shadow the per-class inline branch. const body = await invokeAngularMiddleware( @@ -1373,6 +1389,20 @@ describe('@ng/component endpoint resolves the styles per class', () => { ) } + // The component-file branch queues `pendingHmrUpdates` under `ctx.file` + // verbatim, and the endpoint looks the id up verbatim. Asserting the queued + // id here turns a spelling mismatch into a named failure instead of an + // unexplained empty body — the difference only shows up on Windows, where + // a normalized path and a `join()` path are different strings. + function expectDispatched(mockServer: any, componentId: string) { + const ids = mockServer._wsMessages + .filter((m: any) => m?.event === 'angular:component-update') + .map((m: any) => decodeURIComponent(m.data.id)) + expect(ids, 'expected the HMR dispatch to use the requested path spelling').toContain( + componentId, + ) + } + function getMiddleware(mockServer: any) { const middleware = (mockServer.middlewares.use as ReturnType).mock.calls[0]?.[0] expect(middleware, 'expected middleware to be registered').toBeDefined() @@ -1508,6 +1538,8 @@ describe('@ng/component endpoint resolves the styles per class', () => { const ctx = createMockHmrContext(mixedPath, [{ id: mixedPath }], mockServer) await callHandleHotUpdate(plugin, ctx) + expectDispatched(mockServer, `${mixedPath}@InlineComponent`) + // InlineComponent must get its OWN inline styles — a sibling's styleUrl // making the FILE-level list non-empty must not shadow the inline branch. const body = await invokeAngularMiddleware( @@ -1581,13 +1613,16 @@ describe('@ng/component endpoint resolves the styles per class', () => { const edited = source.replace('color: red', 'color: green') writeFileSync(inlineCommentPath, edited) - const ctx = createMockHmrContext( - normalizePath(inlineCommentPath), - [{ id: normalizePath(inlineCommentPath) }], - mockServer, - ) + // Editing the `.ts` itself takes the component-file branch, which keys + // `componentsByFile` and `pendingHmrUpdates` on `ctx.file` verbatim. Use + // the same spelling here, for `transform` above, and for the endpoint + // request below: on Windows a normalized path and a `join()` path differ + // as strings, and the lookup would miss. Real Vite hands both hooks the + // same normalized spelling, so only a test can mix them. + const ctx = createMockHmrContext(inlineCommentPath, [{ id: inlineCommentPath }], mockServer) await callHandleHotUpdate(plugin, ctx) + expectDispatched(mockServer, `${inlineCommentPath}@InlineCommentComponent`) const body = await invokeAngularMiddleware( getMiddleware(mockServer), `${inlineCommentPath}@InlineCommentComponent`, @@ -1721,4 +1756,220 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).not.toBe('') expect(body).toContain('PS_CONST_MARKER') }) + + it('falls back when the singular `styleUrl` is a same-file constant', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const ownCssPath = join(appDir, 'ps-singconst-own.component.css') + const sibCssPath = join(appDir, 'ps-singconst-sib.component.css') + const singConstPath = join(appDir, 'ps-singconst.component.ts') + writeFileSync(ownCssPath, '.PS_SINGCONST_OWN_MARKER { color: red; }') + writeFileSync(sibCssPath, '.PS_SINGCONST_SIB_MARKER { color: red; }') + + // The Rust extractor folds the const (verified: it reports ./own.css), + // but the text scan cannot. That is "unknown", not "no styles" — serving + // an empty set would strip this component's CSS entirely. + const source = ` + import { Component } from '@angular/core'; + const STYLE_URL = './ps-singconst-own.component.css'; + @Component({ + selector: 'app-ps-singconst', + template: '

const

', + styleUrl: STYLE_URL, + }) + export class SingConstComponent {} + @Component({ + selector: 'app-ps-singconst-sib', + template: '

sib

', + styleUrls: ['./ps-singconst-sib.component.css'], + }) + export class SingConstSiblingComponent {} + ` + writeFileSync(singConstPath, source) + await transformSource(plugin, source, singConstPath) + + writeFileSync(ownCssPath, '.PS_SINGCONST_OWN_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(ownCssPath), + [{ id: normalizePath(ownCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${singConstPath}@SingConstComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_SINGCONST_OWN_MARKER') + }) + + it('serves no styles for an explicitly empty `styleUrls` array', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const sibCssPath = join(appDir, 'ps-emptyurls-sib.component.css') + const emptyUrlsPath = join(appDir, 'ps-emptyurls.component.ts') + writeFileSync(sibCssPath, '.PS_EMPTYURLS_SIB_MARKER { color: red; }') + + // `styleUrls: []` is valid and definitive: this class has no styles. It + // must not inherit the file-level union. + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-emptyurls', + template: '

empty

', + styleUrls: [], + }) + export class EmptyUrlsComponent {} + @Component({ + selector: 'app-ps-emptyurls-sib', + template: '

sib

', + styleUrls: ['./ps-emptyurls-sib.component.css'], + }) + export class EmptyUrlsSiblingComponent {} + ` + writeFileSync(emptyUrlsPath, source) + await transformSource(plugin, source, emptyUrlsPath) + + writeFileSync(sibCssPath, '.PS_EMPTYURLS_SIB_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(sibCssPath), + [{ id: normalizePath(sibCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${emptyUrlsPath}@EmptyUrlsComponent`, + ) + expect(body).not.toBe('') + expect(body).not.toContain('PS_EMPTYURLS_SIB_MARKER') + }) + + it('serves no styles for an explicitly empty inline `styles` array', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const sibCssPath = join(appDir, 'ps-emptyinline-sib.component.css') + const emptyInlinePath = join(appDir, 'ps-emptyinline.component.ts') + writeFileSync(sibCssPath, '.PS_EMPTYINLINE_SIB_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-emptyinline', + template: '

empty

', + styles: [], + }) + export class EmptyInlineComponent {} + @Component({ + selector: 'app-ps-emptyinline-sib', + template: '

sib

', + styleUrls: ['./ps-emptyinline-sib.component.css'], + }) + export class EmptyInlineSiblingComponent {} + ` + writeFileSync(emptyInlinePath, source) + await transformSource(plugin, source, emptyInlinePath) + + writeFileSync(sibCssPath, '.PS_EMPTYINLINE_SIB_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(sibCssPath), + [{ id: normalizePath(sibCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${emptyInlinePath}@EmptyInlineComponent`, + ) + expect(body).not.toBe('') + expect(body).not.toContain('PS_EMPTYINLINE_SIB_MARKER') + }) + + it('falls back when a `styleUrls` array mixes a constant with a literal', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const constCssPath = join(appDir, 'ps-mixed-const.component.css') + const litCssPath = join(appDir, 'ps-mixed-lit.component.css') + const mixedPath = join(appDir, 'ps-mixed.component.ts') + writeFileSync(constCssPath, '.PS_MIXED_CONST_MARKER { color: red; }') + writeFileSync(litCssPath, '.PS_MIXED_LIT_MARKER { color: red; }') + + // One entry is a const the text scan cannot read. Returning just the + // literal would silently drop a stylesheet — the array is unknown, not + // partially known. + const source = ` + import { Component } from '@angular/core'; + const MIXED_STYLE = './ps-mixed-const.component.css'; + @Component({ + selector: 'app-ps-mixed', + template: '

mixed

', + styleUrls: [MIXED_STYLE, './ps-mixed-lit.component.css'], + }) + export class MixedComponent {} + ` + writeFileSync(mixedPath, source) + await transformSource(plugin, source, mixedPath) + + writeFileSync(litCssPath, '.PS_MIXED_LIT_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(litCssPath), + [{ id: normalizePath(litCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${mixedPath}@MixedComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_MIXED_LIT_MARKER') + expect(body).toContain('PS_MIXED_CONST_MARKER') + }) + + it('falls back when a `styleUrl` template literal is interpolated', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const interpCssPath = join(appDir, 'ps-interp.component.css') + const interpPath = join(appDir, 'ps-interp.component.ts') + writeFileSync(interpCssPath, '.PS_INTERP_MARKER { color: red; }') + + // The Rust extractor folds this to ./ps-interp.component.css; the raw + // slice `${DIR}/ps-interp.component.css` is not a real path. + const source = ` + import { Component } from '@angular/core'; + const DIR = '.'; + @Component({ + selector: 'app-ps-interp', + template: '

interp

', + styleUrl: \`\${DIR}/ps-interp.component.css\`, + }) + export class InterpComponent {} + ` + writeFileSync(interpPath, source) + await transformSource(plugin, source, interpPath) + + writeFileSync(interpCssPath, '.PS_INTERP_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(interpCssPath), + [{ id: normalizePath(interpCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${interpPath}@InterpComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_INTERP_MARKER') + }) }) diff --git a/napi/angular-compiler/vite-plugin/index.ts b/napi/angular-compiler/vite-plugin/index.ts index 7e93f5465..3ae01fc69 100644 --- a/napi/angular-compiler/vite-plugin/index.ts +++ b/napi/angular-compiler/vite-plugin/index.ts @@ -1473,16 +1473,17 @@ function extractClassStylesFor( const fields = locateStyleFieldsFor(code, className) if (!fields) return null - let inline: string[] = [] - if (fields.inline) { - inline = readStringLiterals(code, fields.inline) - if (inline.length === 0) return null - } - let urls: string[] = [] - if (fields.urls) { - urls = readStringLiterals(code, fields.urls) - if (urls.length === 0) return null + const read = (field: (typeof fields)['inline']): string[] | null => { + if (field.kind === 'absent') return [] + if (field.kind === 'unreadable') return null + const { literals, complete } = readStringLiterals(code, field.range) + // A partial read is unknown, not partial knowledge: acting on the + // literals alone would drop whatever the unreadable elements name. + return complete ? literals : null } + const inline = read(fields.inline) + const urls = read(fields.urls) + if (inline === null || urls === null) return null return { inline, urls } } @@ -1502,8 +1503,8 @@ function extractClassStylesFor( function extractInlineStyles(code: string, className: string): string[] | null { const range = locateStylesFieldFor(code, className) if (!range) return null - const styles = readStringLiterals(code, range) - return styles.length > 0 ? styles : null + const { literals } = readStringLiterals(code, range) + return literals.length > 0 ? literals : null } /** diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index 585ed84a0..d1cea8ce0 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -203,12 +203,34 @@ export function emptyDelimitedRange(code: string, range: [number, number]): stri * * Returns null if no qualifying field is found. */ -function locateFieldInsideArgs( +/** + * The state of one decorator field, which is three-valued: the key may be + * missing entirely, present with a value this scan can read, or present with + * a value it cannot (an identifier, a call, a concatenation). + * + * The third state is why this exists. Collapsing it into "absent" tells a + * caller the component declares nothing, when in truth the value is simply + * beyond a text scan — the Rust extractor folds same-file constants that this + * cannot. Those two need opposite fallbacks. + */ +export type FieldValue = + | { kind: 'absent' } + | { kind: 'unreadable' } + | { kind: 'literal'; range: [number, number] } + +/** + * Find a top-level field of the `@Component(...)` args and classify its + * value. See `FieldValue` for the three outcomes. + * + * The walk is identical to `locateFieldInsideArgs`, which is expressed on top + * of this; only the reporting differs. + */ +function findFieldInArgs( code: string, argsRange: [number, number], field: string, openerChars: string, -): [number, number] | null { +): FieldValue { const [openParen, closeParen] = argsRange const stack: Ctx[] = ['paren'] let i = openParen + 1 @@ -226,13 +248,25 @@ function locateFieldInsideArgs( while (j < closeParen && WS_RE.test(code[j])) j++ if (j < closeParen && openerChars.includes(code[j])) { const end = findClosingDelim(code, j) - if (end !== -1 && end < closeParen) return [j, end] + if (end !== -1 && end < closeParen) return { kind: 'literal', range: [j, end] } } + // The key is here; its value is not a shape we can read. + return { kind: 'unreadable' } } } i = advanceOneToken(code, i, stack, closeParen) } - return null + return { kind: 'absent' } +} + +function locateFieldInsideArgs( + code: string, + argsRange: [number, number], + field: string, + openerChars: string, +): [number, number] | null { + const found = findFieldInArgs(code, argsRange, field, openerChars) + return found.kind === 'literal' ? found.range : null } /** @@ -483,6 +517,37 @@ export function locateStyleUrlFor(code: string, className: string): [number, num return found ? locateStyleUrlInArgs(code, found.argsRange) : null } +/** + * Whether a template literal's raw contents contain an unescaped `${`, i.e. + * the literal interpolates and its text is not the final value. A leading + * run of backslashes escapes the `$` only when it is odd-length. + */ +function hasInterpolation(raw: string): boolean { + for (let i = raw.indexOf('${'); i !== -1; i = raw.indexOf('${', i + 2)) { + let backslashes = 0 + while (i - 1 - backslashes >= 0 && raw[i - 1 - backslashes] === '\\') backslashes++ + if (backslashes % 2 === 0) return true + } + return false +} + +/** + * The literals read out of a field value, and whether every element was read. + * + * `complete: false` means the value holds something this scan cannot resolve — + * an identifier, a spread, an interpolated template literal, an unterminated + * string. The literals gathered alongside it are NOT a partial answer to act + * on: a caller that used them would silently drop the stylesheets the + * unreadable elements stand for. Treat an incomplete read as unknown. + * + * An empty array is the opposite case: `complete: true` with no literals is a + * definitive "this field declares nothing". + */ +export interface StringLiteralsRead { + literals: string[] + complete: boolean +} + /** * Read the string literals out of a located field value, given the inclusive * `[start, end]` range of its outer delimiters (the shape every `locate*` @@ -494,22 +559,22 @@ export function locateStyleUrlFor(code: string, className: string): [number, num * * Inside the array body, whitespace, commas and comments are skipped, and * each literal is delimited with `findClosingDelim`, so escape sequences and - * apostrophes inside comments cannot be mistaken for delimiters. Anything - * that is not a string literal (an identifier, a spread, a nested array) is - * skipped rather than treated as an entry. + * apostrophes inside comments cannot be mistaken for delimiters. * * Returns the raw inner contents — no unescaping, no trimming — because HMR - * delivers these verbatim. An unterminated literal ends the scan, returning - * whatever was collected before it. + * delivers these verbatim. */ -export function readStringLiterals(code: string, range: [number, number]): string[] { +export function readStringLiterals(code: string, range: [number, number]): StringLiteralsRead { const [start, end] = range if (code[start] !== '[') { // Bare literal — the range already delimits it. - return [code.slice(start + 1, end)] + const raw = code.slice(start + 1, end) + const interpolated = code[start] === '`' && hasInterpolation(raw) + return { literals: interpolated ? [] : [raw], complete: !interpolated } } const literals: string[] = [] + let complete = true let i = start + 1 while (i < end) { const ch = code[i] @@ -525,52 +590,60 @@ export function readStringLiterals(code: string, range: [number, number]): strin if (ch === "'" || ch === '"' || ch === '`') { const close = findClosingDelim(code, i) // Unterminated literal: nothing further can be read reliably. - if (close === -1 || close >= end) break - literals.push(code.slice(i + 1, close)) + if (close === -1 || close >= end) { + complete = false + break + } + const raw = code.slice(i + 1, close) + if (ch === '`' && hasInterpolation(raw)) { + complete = false + } else { + literals.push(raw) + } i = close + 1 continue } - // Not a literal (identifier, spread, nested array…) — skip this - // character. Every branch above advances `i`, so the scan terminates. + // An identifier, spread, call or nested array: this element cannot be + // read, so the array as a whole is unknown. Every branch above advances + // `i`, so the scan terminates. + complete = false i++ } - return literals + return { literals, complete } } /** - * The value ranges of the style-related fields on one `@Component(...)`. - * A null member means the decorator does not declare that field at all — - * which is different from declaring it with a value no literal can be read - * from (a constant, an identifier), where the range is present but - * `readStringLiterals` returns nothing. + * The style-related fields of one `@Component(...)`, each classified by + * `FieldValue`: absent, unreadable, or a literal range. */ -export interface ClassStyleFieldRanges { +export interface ClassStyleFields { /** `styleUrls: [...]`, else the singular `styleUrl: '...'`. */ - urls: [number, number] | null + urls: FieldValue /** Inline `styles: [...] | '...'`. */ - inline: [number, number] | null + inline: FieldValue } /** - * Locate the style fields of the `@Component(...)` decorating `className`. + * Classify the style fields of the `@Component(...)` decorating `className`. * - * Returns null when no such decorator could be located, so a caller can tell - * "this class declares no styles" (both members null) from "this class could - * not be read" — the two need opposite fallbacks. + * Returns null when no such decorator could be located at all. Otherwise each + * member says whether the class declares that field, and whether its value + * can be read — see `FieldValue`, and note that "declares nothing" and + * "cannot be read" need opposite fallbacks at the call site. * - * `styleUrls` wins over `styleUrl` if a decorator somehow carries both; - * Angular itself rejects that combination, so this only makes the choice - * deterministic. One decorator scan serves all three lookups. + * `styleUrls` wins over `styleUrl` when a decorator carries both, unless the + * plural form is absent; Angular itself rejects that combination, so this only + * makes the choice deterministic. One decorator scan serves all three lookups. */ -export function locateStyleFieldsFor( - code: string, - className: string, -): ClassStyleFieldRanges | null { +export function locateStyleFieldsFor(code: string, className: string): ClassStyleFields | null { const found = locateComponentDecorators(code).find((d) => d.className === className) if (!found) return null + const plural = findFieldInArgs(code, found.argsRange, 'styleUrls', STYLES_OPENERS) return { urls: - locateStyleUrlsInArgs(code, found.argsRange) ?? locateStyleUrlInArgs(code, found.argsRange), - inline: locateStylesInArgs(code, found.argsRange), + plural.kind === 'absent' + ? findFieldInArgs(code, found.argsRange, 'styleUrl', TEMPLATE_OPENERS) + : plural, + inline: findFieldInArgs(code, found.argsRange, 'styles', STYLES_OPENERS), } } From 90edef81d29d5cddcf602d7c9460e3e9bb587d68 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 18:18:50 +0800 Subject: [PATCH 04/10] fix(vite): read quoted metadata keys, and never guess past an unreadable one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@Component({ 'styleUrls': [...] })` is valid TS, and the Rust extractor resolves it, but the text scan saw no field and the classifier reported a confident "this class declares no styles" — so the endpoint served none and the component lost its CSS. Before this branch's styleless fix the case fell back and kept working. Same failure mode as the const case in 9066e43, different trigger. Quoted keys, single and double, are now matched like bare ones. That widens template/templateUrl/styles too; the cross-match guards are unaffected, since only the key's spelling changed, not its identity. The general form matters more than the trigger: "no styles" is only safe to conclude after seeing every top-level key. A computed key or an escaped one hides a field that may exist, so the style fields become unreadable and the endpoint falls back. A field that IS visible stays readable — a decorator holding one computed key does not lose the fields beside it. A spread is deliberately NOT treated that way. Measured: the Rust extractor drops `...BASE` entirely and reports no styleUrls, so the compiled component really has none. Falling back would hand that class its siblings' stylesheets, which is the contamination this PR removes. Serving nothing is what matches the compile path, and a test guards it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 74 ++++++++ .../test/hmr-hot-update.test.ts | 178 ++++++++++++++++++ .../vite-plugin/utils/decorator-fields.ts | 102 +++++++++- 3 files changed, 350 insertions(+), 4 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index c872cfa44..2855dbfcb 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -437,6 +437,80 @@ describe('decorator-fields utils', () => { inline: { kind: 'absent' }, }) }) + + // Quoted keys are valid TS and the Rust extractor resolves them + // (verified: `'styleUrls'` and `"styleUrls"` both report their URL). + // Reading them as absent tells the caller the class declares no + // styles, which strips the component's CSS. + it('reads a single-quoted key the same as the bare form', () => { + const src = `@Component({ 'styleUrls': ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('reads a double-quoted key the same as the bare form', () => { + const src = `@Component({ "styleUrls": ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('reads a quoted singular `styleUrl` key', () => { + const src = `@Component({ 'styleUrl': './solo.css' })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./solo.css']) + }) + + it('reads a quoted inline `styles` key', () => { + const src = `@Component({ 'styles': ['.x{}'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.inline)).toEqual(['.x{}']) + }) + + it('keeps the cross-match guards for quoted keys', () => { + const urls = `@Component({ 'styleUrls': ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(urls, 'Foo')!.inline).toEqual({ kind: 'absent' }) + const inline = `@Component({ 'styles': ['.x{}'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(inline, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + // A computed key hides the field name from this scan, but the Rust + // extractor resolves it (verified: `[K]: ['./computed.css']` reports + // the URL). "Absent" would be a lie, so the whole classification is + // unknown and the caller falls back. + it('reports both fields unreadable when a computed key is present', () => { + const src = `@Component({ [K]: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ + urls: { kind: 'unreadable' }, + inline: { kind: 'unreadable' }, + }) + }) + + it('reports both fields unreadable for a quoted key carrying an escape', () => { + // `'style\u0055rls'` is `styleUrls` after decoding, which the Rust + // extractor does and this scan deliberately does not. + const src = `@Component({ 'style\\u0055rls': ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ + urls: { kind: 'unreadable' }, + inline: { kind: 'unreadable' }, + }) + }) + + it('does not promote a readable field to unreadable because of a computed key', () => { + // The visible field is still exactly what it says; only the fields + // this scan cannot see are unknown. + const src = `@Component({ [K]: 1, styleUrls: ['./a.css'] })\nexport class Foo {}` + const fields = locateStyleFieldsFor(src, 'Foo')! + expect(literalsIn(src, fields.urls)).toEqual(['./a.css']) + expect(fields.inline).toEqual({ kind: 'unreadable' }) + }) + + // A spread is the one unreadable form the Rust extractor ALSO drops + // (verified: `...BASE` reports no styleUrls). Both sides see nothing, + // so "absent" matches what the compiled component gets; falling back + // would hand this class its siblings' CSS. + it('leaves fields absent for a spread, which the compiler also drops', () => { + const src = `@Component({ ...BASE })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ + urls: { kind: 'absent' }, + inline: { kind: 'absent' }, + }) + }) }) // ----------------------------------------------------------------- diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index dc6d94096..10d67213e 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -1972,4 +1972,182 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).not.toBe('') expect(body).toContain('PS_INTERP_MARKER') }) + + // Quoted metadata keys are valid TS, and the Rust extractor resolves + // them (verified: `'styleUrls'` reports its URL). Reading the key as + // absent makes this class look styleless, which serves it nothing. + it('serves the own styleUrls of a class whose key is single-quoted', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const quotedCssPath = join(appDir, 'ps-quoted-own.component.css') + const quotedSibCssPath = join(appDir, 'ps-quoted-sib.component.css') + const quotedPath = join(appDir, 'ps-quoted.component.ts') + writeFileSync(quotedCssPath, '.PS_QUOTED_OWN_MARKER { color: red; }') + writeFileSync(quotedSibCssPath, '.PS_QUOTED_SIB_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-quoted', + template: '

quoted

', + 'styleUrls': ['./ps-quoted-own.component.css'], + }) + export class QuotedKeyComponent {} + @Component({ + selector: 'app-ps-quoted-sib', + template: '

sib

', + styleUrls: ['./ps-quoted-sib.component.css'], + }) + export class QuotedKeySiblingComponent {} + ` + writeFileSync(quotedPath, source) + await transformSource(plugin, source, quotedPath) + + writeFileSync(quotedCssPath, '.PS_QUOTED_OWN_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(quotedCssPath), + [{ id: normalizePath(quotedCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${quotedPath}@QuotedKeyComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_QUOTED_OWN_MARKER') + expect(body).not.toContain('PS_QUOTED_SIB_MARKER') + }) + + it('serves the own styleUrls of a class whose key is double-quoted', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const dqCssPath = join(appDir, 'ps-dquoted-own.component.css') + const dqSibCssPath = join(appDir, 'ps-dquoted-sib.component.css') + const dqPath = join(appDir, 'ps-dquoted.component.ts') + writeFileSync(dqCssPath, '.PS_DQUOTED_OWN_MARKER { color: red; }') + writeFileSync(dqSibCssPath, '.PS_DQUOTED_SIB_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-dquoted', + template: '

dquoted

', + "styleUrls": ['./ps-dquoted-own.component.css'], + }) + export class DQuotedKeyComponent {} + @Component({ + selector: 'app-ps-dquoted-sib', + template: '

sib

', + styleUrls: ['./ps-dquoted-sib.component.css'], + }) + export class DQuotedKeySiblingComponent {} + ` + writeFileSync(dqPath, source) + await transformSource(plugin, source, dqPath) + + writeFileSync(dqCssPath, '.PS_DQUOTED_OWN_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(dqCssPath), + [{ id: normalizePath(dqCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${dqPath}@DQuotedKeyComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_DQUOTED_OWN_MARKER') + expect(body).not.toContain('PS_DQUOTED_SIB_MARKER') + }) + + // A computed key hides the field from this scan while the Rust extractor + // still resolves it, so the class must fall back rather than be served + // nothing — the file-level list carries its stylesheet. + it('falls back when a style field is declared under a computed key', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const computedCssPath = join(appDir, 'ps-computed-own.component.css') + const computedPath = join(appDir, 'ps-computed.component.ts') + writeFileSync(computedCssPath, '.PS_COMPUTED_OWN_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + const K = 'styleUrls'; + @Component({ + selector: 'app-ps-computed', + template: '

computed

', + [K]: ['./ps-computed-own.component.css'], + }) + export class ComputedKeyComponent {} + ` + writeFileSync(computedPath, source) + await transformSource(plugin, source, computedPath) + + writeFileSync(computedCssPath, '.PS_COMPUTED_OWN_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(computedCssPath), + [{ id: normalizePath(computedCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${computedPath}@ComputedKeyComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_COMPUTED_OWN_MARKER') + }) + + // A spread is the one unreadable form the compiler ALSO drops, so + // "declares no styles" is what the compiled component actually gets. + // Falling back here would hand this class its sibling's CSS instead. + it('serves no styles for a spread-only decorator, matching the compiler', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const spreadSibCssPath = join(appDir, 'ps-spread-sib.component.css') + const spreadPath = join(appDir, 'ps-spread.component.ts') + writeFileSync(spreadSibCssPath, '.PS_SPREAD_SIB_MARKER { color: red; }') + + const source = ` + import { Component } from '@angular/core'; + const BASE = { selector: 'app-ps-spread' }; + @Component({ + template: '

spread

', + ...BASE, + }) + export class SpreadComponent {} + @Component({ + selector: 'app-ps-spread-sib', + template: '

sib

', + styleUrls: ['./ps-spread-sib.component.css'], + }) + export class SpreadSiblingComponent {} + ` + writeFileSync(spreadPath, source) + await transformSource(plugin, source, spreadPath) + + writeFileSync(spreadSibCssPath, '.PS_SPREAD_SIB_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(spreadSibCssPath), + [{ id: normalizePath(spreadSibCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${spreadPath}@SpreadComponent`, + ) + expect(body).not.toBe('') + expect(body).not.toContain('PS_SPREAD_SIB_MARKER') + }) }) diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index d1cea8ce0..cf6a6fefe 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -240,8 +240,10 @@ function findFieldInArgs( // valid at the @Component's immediate object-literal depth // (`['paren', 'brace']`) — anything deeper is a nested literal that // isn't the component's metadata. - if (stack.length === 2 && stack[1] === 'brace' && isFieldKeyAt(code, i, field, closeParen)) { - let j = i + field.length + const afterKey = + stack.length === 2 && stack[1] === 'brace' ? matchFieldKeyAt(code, i, field, closeParen) : -1 + if (afterKey !== -1) { + let j = afterKey while (j < closeParen && WS_RE.test(code[j])) j++ if (code[j] === ':') { j++ @@ -269,6 +271,26 @@ function locateFieldInsideArgs( return found.kind === 'literal' ? found.range : null } +/** + * If a key for `field` starts at `position`, return the index just past it; + * otherwise -1. Accepts the bare form (`styleUrls:`) and the quoted forms + * (`'styleUrls':`, `"styleUrls":`), which are valid TS and which the Rust + * extractor resolves. + * + * A quoted key must match `field` character for character. One written with + * an escape (`'style\u0055rls'`) is deliberately not decoded here — see + * `hasUnreadableKey`, which reports it as beyond this scan rather than + * letting it read as a key that isn't there. + */ +function matchFieldKeyAt(code: string, position: number, field: string, limit: number): number { + if (isFieldKeyAt(code, position, field, limit)) return position + field.length + const quote = code[position] + if (quote !== "'" && quote !== '"') return -1 + const close = findClosingDelim(code, position) + if (close === -1 || close >= limit) return -1 + return code.slice(position + 1, close) === field ? close + 1 : -1 +} + /** * Whether `field` starts at `position` in `code` AND is bounded on both sides * by non-identifier characters (so `template` doesn't match the start of @@ -638,12 +660,84 @@ export interface ClassStyleFields { export function locateStyleFieldsFor(code: string, className: string): ClassStyleFields | null { const found = locateComponentDecorators(code).find((d) => d.className === className) if (!found) return null + // A key this scan cannot read could BE a style field, so "absent" would be + // a guess rather than an answer. Only absent is promoted: a field we did + // read says exactly what it says, whatever else the object holds. + const blind = hasUnreadableKey(code, found.argsRange) + const classify = (value: FieldValue): FieldValue => + blind && value.kind === 'absent' ? { kind: 'unreadable' } : value + const plural = findFieldInArgs(code, found.argsRange, 'styleUrls', STYLES_OPENERS) return { - urls: + urls: classify( plural.kind === 'absent' ? findFieldInArgs(code, found.argsRange, 'styleUrl', TEMPLATE_OPENERS) : plural, - inline: findFieldInArgs(code, found.argsRange, 'styles', STYLES_OPENERS), + ), + inline: classify(findFieldInArgs(code, found.argsRange, 'styles', STYLES_OPENERS)), } } + +/** + * Whether the decorator's own object literal holds a key this scan cannot + * resolve to a name, which makes "the class declares no styles" unknowable. + * + * Two forms qualify, both of which the Rust extractor DOES resolve, so + * reading them as absent would strip a component's CSS: + * - a computed key (`[K]: [...]`); + * - a quoted key carrying an escape (`'style\u0055rls'`), which this scan + * matches literally and so would miss. + * + * A spread (`...BASE`) deliberately does NOT qualify. The Rust extractor + * drops it too, so the compiled component genuinely has no styles from it; + * treating the class as unknown would hand it its siblings' stylesheets + * instead, which is the contamination this classification exists to avoid. + */ +function hasUnreadableKey(code: string, argsRange: [number, number]): boolean { + const [openParen, closeParen] = argsRange + const stack: Ctx[] = ['paren'] + let i = openParen + 1 + // Position within the decorator's own object literal. Keys sit at the + // start and after each comma; `:` hands over to the value. + let atKey = true + + while (i < closeParen) { + if (stack.length === 2 && stack[1] === 'brace') { + const ch = code[i] + if (WS_RE.test(ch)) { + i++ + continue + } + const afterComment = skipComment(code, i, closeParen) + if (afterComment !== -1) { + i = afterComment + continue + } + if (ch === ',') { + atKey = true + i++ + continue + } + if (ch === ':') { + atKey = false + i++ + continue + } + if (atKey) { + // `[` here opens a computed key, not an array value: a value only + // follows a `:`, which would have cleared `atKey`. + if (ch === '[') return true + if (ch === "'" || ch === '"') { + const close = findClosingDelim(code, i) + if (close === -1 || close >= closeParen) return true + if (code.slice(i + 1, close).includes('\\')) return true + atKey = false + i = close + 1 + continue + } + } + } + i = advanceOneToken(code, i, stack, closeParen) + } + return false +} From c2342b049065f217dc2ee14bd4a14ede52cc25b9 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 18:29:48 +0800 Subject: [PATCH 05/10] fix(vite): cook style literals, and ignore decorators inside comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more ways the per-class scan disagreed with the Rust extractor, both ending in a component served the wrong CSS or none. Escapes: the scanner returned raw source text and called the read complete, so `styleUrls: ['./cmp.css']` produced a path containing the literal escape, the read failed, the per-style catch swallowed it, and the component lost its stylesheet. Literals are now cooked with JS semantics — single-char escapes, \xHH, \uHHHH, \u{…}, line continuations, and NonEscapeCharacter — matching what Rust resolves. Malformed hex or unicode, a trailing backslash, and legacy octal report incomplete so the caller falls back; we are exact where we can be and defer where we cannot. Literals with no backslash take a fast path and are byte-for-byte unchanged. Decoding was chosen over marking escaped literals unknown: falling back hands a multi-component file the sibling union, which is the very contamination this change removes. Phantom decorators: `locateComponentDecorators` enumerated `@Component(` with a regex, so a commented-out decorator between a real one and its class captured the class. The endpoint then read the commented metadata — serving old.css instead of real.css, or nothing at all when the phantom also hid the template. Enumeration now skips comments and string literals. Narrower than it first appeared: only a phantom sitting between a decorator and its class did damage; one appearing first was already dropped by the second pass, and both orderings are now pinned. Two tests from earlier in this branch asserted the raw text — the defect itself — and now assert the cooked value. Verified against Rust: `./it's.css`, `./cmp.css`, `./aqb.css`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 126 +++++++++++- .../test/hmr-hot-update.test.ts | 95 +++++++++ .../vite-plugin/utils/decorator-fields.ts | 184 ++++++++++++++++-- 3 files changed, 381 insertions(+), 24 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index 2855dbfcb..4fb933915 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -59,6 +59,79 @@ describe('decorator-fields utils', () => { expect(locateComponentDecorators(src)).toEqual([]) }) + it('ignores a commented-out decorator that follows the real one', () => { + // The phantom sits between the real decorator and the class. Pairing + // the class with it would read the stale metadata, not merely miss it. + const src = [ + `@Component({ selector: 'x', styleUrls: ['./real.css'] })`, + `// @Component({ styleUrl: './old.css' })`, + `export class FooComponent {}`, + ].join('\n') + const out = locateComponentDecorators(src) + expect(out).toHaveLength(1) + expect(out[0].className).toBe('FooComponent') + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'FooComponent')!).literals).toEqual([ + './real.css', + ]) + }) + + it('ignores a decorator inside a block comment that follows the real one', () => { + const src = [ + `@Component({ selector: 'x', styleUrls: ['./real.css'] })`, + `/* @Component({ styleUrl: './old.css' }) */`, + `export class FooComponent {}`, + ].join('\n') + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'FooComponent')!).literals).toEqual([ + './real.css', + ]) + }) + + it('ignores a commented-out decorator that precedes the real one', () => { + const src = [ + `// @Component({ styleUrl: './old.css' })`, + `@Component({ selector: 'x', styleUrls: ['./real.css'] })`, + `export class FooComponent {}`, + ].join('\n') + const out = locateComponentDecorators(src) + expect(out).toHaveLength(1) + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'FooComponent')!).literals).toEqual([ + './real.css', + ]) + }) + + it('ignores a decorator written inside a string literal', () => { + const src = [ + `const doc = 'see @Component({ styleUrl: "./str.css" }) for details';`, + `@Component({ selector: 'x', styleUrls: ['./real.css'] })`, + `export class FooComponent {}`, + ].join('\n') + const out = locateComponentDecorators(src) + expect(out).toHaveLength(1) + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'FooComponent')!).literals).toEqual([ + './real.css', + ]) + }) + + it('pairs each class with its own decorator when a phantom sits between them', () => { + const src = [ + `@Component({ selector: 'a', styleUrls: ['./a.css'] })`, + `// @Component({ styleUrl: './fake.css' })`, + `export class AComponent {}`, + `@Component({ selector: 'b', styleUrls: ['./b.css'] })`, + `export class BComponent {}`, + ].join('\n') + expect(locateComponentDecorators(src).map((d) => d.className)).toEqual([ + 'AComponent', + 'BComponent', + ]) + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'AComponent')!).literals).toEqual([ + './a.css', + ]) + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'BComponent')!).literals).toEqual([ + './b.css', + ]) + }) + it('returns a single entry for a single-component file', () => { const src = `@Component({ selector: 'x' })\nexport class FooComponent {}` const out = locateComponentDecorators(src) @@ -560,8 +633,52 @@ describe('decorator-fields utils', () => { expect(readOf(`[]`)).toEqual({ literals: [], complete: true }) }) - it('keeps an escaped quote inside a literal, unescaped', () => { - expect(literalsOf(`['it\\'s.css']`)).toEqual([`it\\'s.css`]) + it('decodes hex and unicode escapes to the value they denote', () => { + // The Rust extractor reports the cooked value; a raw read would name a + // path that does not exist. + expect(literalsOf(String.raw`['.\x2fa.css', '\u{2e}/b.css']`)).toEqual(['./a.css', './b.css']) + }) + + it('decodes the standard single-character escapes', () => { + expect(literalsOf(String.raw`['a\nb\tc\rd\be\ff\vg']`)).toEqual(['a\nb\tc\rd\be\ff\vg']) + }) + + it('decodes a backslash escape to one backslash', () => { + expect(literalsOf(String.raw`['a\\b']`)).toEqual([String.raw`a\b`]) + }) + + it('decodes an unrecognized escape to the character itself', () => { + // `\q` is a NonEscapeCharacter: it denotes `q`, which is what the Rust + // extractor reports too. + expect(literalsOf(String.raw`['./a\qb.css']`)).toEqual(['./aqb.css']) + }) + + it('drops a line continuation', () => { + expect(literalsOf("['a\\\nb']")).toEqual(['ab']) + }) + + it('reports a truncated unicode escape as incomplete', () => { + expect(readOf(String.raw`['./a\u12']`)).toEqual({ literals: [], complete: false }) + }) + + it('reports a malformed hex escape as incomplete', () => { + expect(readOf(String.raw`['./a\xZZ']`)).toEqual({ literals: [], complete: false }) + }) + + it('reports a legacy octal escape as incomplete', () => { + // Illegal in a module; guessing a value would be worse than falling back. + expect(readOf(String.raw`['./a\101']`)).toEqual({ literals: [], complete: false }) + }) + + it('leaves a literal with no escapes byte for byte unchanged', () => { + const css = `a::before { content: "x"; } [data-x="y"] { color: red; }` + expect(literalsOf(`['${css}']`)).toEqual([css]) + }) + + it('decodes an escaped quote inside a literal', () => { + // The literal denotes `it's.css`, which is what the Rust extractor + // reports and what has to be resolved against the filesystem. + expect(literalsOf(`['it\\'s.css']`)).toEqual([`it's.css`]) }) it('ignores an apostrophe inside a block comment before an entry', () => { @@ -597,8 +714,11 @@ describe('decorator-fields utils', () => { }) it('treats an escaped `${` in a template literal as ordinary text', () => { + // `hasInterpolation` reads the odd backslash as an escape, so the + // literal is complete; decoding then resolves `\$` to `$`, leaving the + // `${…}` as the literal text it denotes. The two must agree. expect(readOf('[`\\${NOT_INTERPOLATED}.css`]')).toEqual({ - literals: ['\\${NOT_INTERPOLATED}.css'], + literals: ['${NOT_INTERPOLATED}.css'], complete: true, }) }) diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index 10d67213e..009340ea0 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -2150,4 +2150,99 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).not.toBe('') expect(body).not.toContain('PS_SPREAD_SIB_MARKER') }) + it('serves a styleUrls entry written with a JavaScript escape', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const escCssPath = join(appDir, 'ps-esc.component.css') + const escSibCssPath = join(appDir, 'ps-esc-sib.component.css') + const escPath = join(appDir, 'ps-esc.component.ts') + writeFileSync(escCssPath, '.PS_ESC_OWN_MARKER { color: red; }') + writeFileSync(escSibCssPath, '.PS_ESC_SIB_MARKER { color: blue; }') + + // `\u002e` is `.` — the cooked value is `./ps-esc.component.css`, which + // is what the Rust extractor sees. A raw read yields a path that does + // not exist, and the per-style catch would swallow the failure. + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-esc', + template: '

esc

', + styleUrls: ['\\u002e/ps-esc\\u002ecomponent.css'], + }) + export class EscComponent {} + @Component({ + selector: 'app-ps-esc-sib', + template: '

sib

', + styleUrls: ['./ps-esc-sib.component.css'], + }) + export class EscSiblingComponent {} + ` + writeFileSync(escPath, source) + await transformSource(plugin, source, escPath) + + writeFileSync(escSibCssPath, '.PS_ESC_SIB_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(escSibCssPath), + [{ id: normalizePath(escSibCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware(getMiddleware(mockServer), `${escPath}@EscComponent`) + expect(body).not.toBe('') + expect(body).toContain('PS_ESC_OWN_MARKER') + expect(body).not.toContain('PS_ESC_SIB_MARKER') + }) + + it('ignores a commented-out decorator when picking the styles of a class', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const realCssPath = join(appDir, 'ps-cmt-real.component.css') + const oldCssPath = join(appDir, 'ps-cmt-old.component.css') + const cmtSibCssPath = join(appDir, 'ps-cmt-sib.component.css') + const cmtPath = join(appDir, 'ps-cmt.component.ts') + writeFileSync(realCssPath, '.PS_CMT_REAL_MARKER { color: red; }') + writeFileSync(oldCssPath, '.PS_CMT_OLD_MARKER { color: blue; }') + writeFileSync(cmtSibCssPath, '.PS_CMT_SIB_MARKER { color: teal; }') + + // A leftover commented decorator sits between the real one and the class. + // Enumeration must not pair the class with the commented occurrence. + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-cmt', + template: '

cmt

', + styleUrls: ['./ps-cmt-real.component.css'], + }) + // @Component({ styleUrl: './ps-cmt-old.component.css' }) + export class CommentedComponent {} + @Component({ + selector: 'app-ps-cmt-sib', + template: '

sib

', + styleUrls: ['./ps-cmt-sib.component.css'], + }) + export class CommentedSiblingComponent {} + ` + writeFileSync(cmtPath, source) + await transformSource(plugin, source, cmtPath) + + writeFileSync(cmtSibCssPath, '.PS_CMT_SIB_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(cmtSibCssPath), + [{ id: normalizePath(cmtSibCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${cmtPath}@CommentedComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_CMT_REAL_MARKER') + expect(body).not.toContain('PS_CMT_OLD_MARKER') + expect(body).not.toContain('PS_CMT_SIB_MARKER') + }) }) diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index cf6a6fefe..b1344aa9a 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -21,9 +21,11 @@ * literal `@Component` form is recognized. * - **Parenthesized decorator expressions** like `@(Component as any)(...)` * — uncommon and not supported. - * - **Quoted (`{ 'styles': [...] }`) or computed (`{ ['styles']: [...] }`) - * property keys** for the `styles`/`template` field. Almost never used - * in Angular; locator returns null for such forms. + * - **Computed property keys** (`{ ['styles']: [...] }`) and quoted keys + * carrying an escape (`{ 'style\u0055rls': [...] }`) can't be resolved + * to a name here. Plain quoted keys (`{ 'styles': [...] }`) are matched; + * the unresolvable forms are reported so the caller can fall back rather + * than read them as a field that isn't there. * - **Concatenated style strings** inside an array (`styles: ['a' + 'b']`) * are extracted as two separate elements; cosmetic but harmless because * the browser sees the same CSS either way. @@ -64,6 +66,9 @@ const ASCII_WORD_RE = /[A-Za-z0-9_$]/ const IDENT_START_RE = /[\p{L}_$]/u const IDENT_CONT_RE = /[\p{L}\p{N}_$]/u +/** The only decorator form recognized — see the module docstring. */ +const DECORATOR_NAME = 'Component' + /** Opener chars accepted as the value of `styles:` — `string | string[]`. */ const STYLES_OPENERS = '\'"`[' /** Opener chars accepted as the value of `template:` — just string literals. */ @@ -322,26 +327,50 @@ export interface ComponentDecorator { * class (dangling, malformed, anonymous) are skipped — the caller sees only * well-formed component declarations. * - * The class-name scan is bounded between this decorator's closing `)` and - * either the next `@Component\s*\(` or end of file. That bound prevents one - * decorator's scan from accidentally consuming a sibling's class identifier, - * and combined with the comment- and string-aware walkers in - * `findClosingDelim` and `findClassName` it correctly handles `@Component` - * occurrences inside comments, strings, and template literals. + * Enumeration itself skips comments and string/template literals, so a + * commented-out or quoted `@Component(` is never treated as a decorator. + * The class-name scan is then bounded between one decorator's closing `)` + * and the next decorator's `@`, which stops it consuming a sibling's class + * identifier — a bound that is only correct because phantom occurrences + * were excluded first. * * See the module-level docstring for a full list of known limitations. */ export function locateComponentDecorators(code: string): ComponentDecorator[] { - // Pass 1: find every `@Component(...)` and bound its args list. + // Pass 1: find every `@Component(...)` and bound its args list. The walk + // skips comments and string/template literals, so a commented-out or + // quoted `@Component(` is not mistaken for a real decorator. That matters + // beyond ignoring it: a phantom occurrence between a real decorator and + // its class would bound the real one's class-name scan in pass 2, leaving + // the class paired with the phantom's metadata. type Found = { decoratorStart: number; openParen: number; closeParen: number } - const decoratorRe = /@Component\s*\(/g const found: Found[] = [] - let m: RegExpExecArray | null - while ((m = decoratorRe.exec(code)) !== null) { - const openParen = m.index + m[0].length - 1 - const closeParen = findClosingDelim(code, openParen) - if (closeParen === -1) continue - found.push({ decoratorStart: m.index, openParen, closeParen }) + let i = 0 + while (i < code.length) { + const afterComment = skipComment(code, i, code.length) + if (afterComment !== -1) { + i = afterComment + continue + } + const ch = code[i] + if (ch === "'" || ch === '"' || ch === '`') { + const close = findClosingDelim(code, i) + i = close === -1 ? code.length : close + 1 + continue + } + if (ch === '@' && code.startsWith(DECORATOR_NAME, i + 1)) { + let j = i + 1 + DECORATOR_NAME.length + while (j < code.length && WS_RE.test(code[j])) j++ + if (code[j] === '(') { + const closeParen = findClosingDelim(code, j) + if (closeParen !== -1) { + found.push({ decoratorStart: i, openParen: j, closeParen }) + i = closeParen + 1 + continue + } + } + } + i++ } // Pass 2: for each decorator, scan forward from its `)` to either the next @@ -553,6 +582,115 @@ function hasInterpolation(raw: string): boolean { return false } +const SINGLE_CHAR_ESCAPES: Record = { + n: '\n', + t: '\t', + r: '\r', + b: '\b', + f: '\f', + v: '\v', +} + +/** Line terminators that a backslash may continue across. */ +const LINE_TERMINATORS = new Set(['\n', '\r', '
', '
']) + +const HEX_ONLY_RE = /^[0-9a-fA-F]+$/ + +/** + * Decode the escape sequences in a string- or template-literal's raw source + * text into the value it actually denotes, or null if the text holds an + * escape this cannot resolve. + * + * The raw text is what the source spells; the decoded text is what the Rust + * extractor sees and what a path or a stylesheet must be compared against. + * `'./a.css'` names `./a.css`, and reading it raw yields a path that + * does not exist. + * + * Null is returned for a malformed escape (`\xZZ`, a truncated `\u12`) and + * for the legacy octal forms, which are outright errors in a module. Those + * are cases where guessing a value would be worse than telling the caller + * this literal is beyond the scan. + * + * A raw text with no backslash is returned unchanged, so the overwhelmingly + * common literal costs nothing and reads byte for byte as before. + */ +function decodeEscapes(raw: string): string | null { + if (!raw.includes('\\')) return raw + + let out = '' + let i = 0 + while (i < raw.length) { + const ch = raw[i] + if (ch !== '\\') { + out += ch + i++ + continue + } + + const next = raw[i + 1] + // A trailing backslash cannot occur in a well-delimited literal, since + // it would have escaped the closing quote. + if (next === undefined) return null + + // Line continuation: the backslash and the terminator both vanish. + if (LINE_TERMINATORS.has(next)) { + i += next === '\r' && raw[i + 2] === '\n' ? 3 : 2 + continue + } + + const single = SINGLE_CHAR_ESCAPES[next] + if (single !== undefined) { + out += single + i += 2 + continue + } + + if (next === 'x') { + const hex = raw.slice(i + 2, i + 4) + if (hex.length < 2 || !HEX_ONLY_RE.test(hex)) return null + out += String.fromCharCode(parseInt(hex, 16)) + i += 4 + continue + } + + if (next === 'u') { + if (raw[i + 2] === '{') { + const close = raw.indexOf('}', i + 3) + if (close === -1) return null + const hex = raw.slice(i + 3, close) + if (!HEX_ONLY_RE.test(hex)) return null + const code = parseInt(hex, 16) + if (code > 0x10ffff) return null + out += String.fromCodePoint(code) + i = close + 1 + continue + } + const hex = raw.slice(i + 2, i + 6) + if (hex.length < 4 || !HEX_ONLY_RE.test(hex)) return null + out += String.fromCharCode(parseInt(hex, 16)) + i += 6 + continue + } + + // `\0` is NUL only when no digit follows; with one it is a legacy octal + // escape, which a module rejects outright. `\1`-`\9` likewise. + if (next >= '0' && next <= '9') { + if (next === '0' && !(raw[i + 2] >= '0' && raw[i + 2] <= '9')) { + out += '\0' + i += 2 + continue + } + return null + } + + // Anything else denotes itself: `\\`, `\'`, `\"`, `` \` ``, `\$`, and + // any other non-escape character. + out += next + i += 2 + } + return out +} + /** * The literals read out of a field value, and whether every element was read. * @@ -591,8 +729,11 @@ export function readStringLiterals(code: string, range: [number, number]): Strin if (code[start] !== '[') { // Bare literal — the range already delimits it. const raw = code.slice(start + 1, end) - const interpolated = code[start] === '`' && hasInterpolation(raw) - return { literals: interpolated ? [] : [raw], complete: !interpolated } + if (code[start] === '`' && hasInterpolation(raw)) return { literals: [], complete: false } + const decoded = decodeEscapes(raw) + return decoded === null + ? { literals: [], complete: false } + : { literals: [decoded], complete: true } } const literals: string[] = [] @@ -617,10 +758,11 @@ export function readStringLiterals(code: string, range: [number, number]): Strin break } const raw = code.slice(i + 1, close) - if (ch === '`' && hasInterpolation(raw)) { + const decoded = ch === '`' && hasInterpolation(raw) ? null : decodeEscapes(raw) + if (decoded === null) { complete = false } else { - literals.push(raw) + literals.push(decoded) } i = close + 1 continue From 3a43c73edecc83697ba4b1a2f1555584ace6e902 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 18:39:27 +0800 Subject: [PATCH 06/10] fix(vite): treat a shorthand styleUrl as unreadable, and audit the key forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shorthand property matched the key but had no colon, so the lookup fell through and the classifier concluded "definitively styleless" — the endpoint served nothing while Rust resolved the constant: const styleUrl = './x.css' @Component({ template: '

', styleUrl }) rust ./x.css, scan absent This was the fourth round fixing one form of the same defect, each found by a reviewer rather than by us. So rather than patch shorthand alone, 26 key and value forms were probed against extractComponentUrls. Exactly one was broken; 10 of the new guards passed on first run, which is what makes the audit credible rather than selective. Calibration matters as much as the fix. Reporting unreadable too eagerly sends the class to the file-level union, which is the sibling contamination this work removes. So findFieldInArgs now tracks key versus value position — without it the fix would have degraded `selector: styleUrl`, a readable field sitting right there — and only the SINGULAR styleUrl shorthand is unreadable, because that is the only form the compiler resolves. Unrelated methods, getters, and deeper nesting are untouched. Three divergences are deliberately left alone. Duplicate keys are a TypeScript error, so the input is invalid either way. For a numeric key and an `as` cast it is Rust that returns nothing while our scan reads the literal — matching it would mean dropping styles to mirror a parsing gap on the other side, which is the wrong direction to converge. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 71 +++++++++++++++ .../test/hmr-hot-update.test.ts | 52 +++++++++++ .../vite-plugin/utils/decorator-fields.ts | 89 +++++++++++++++---- 3 files changed, 195 insertions(+), 17 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index 4fb933915..5b3be6dc9 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -584,6 +584,77 @@ describe('decorator-fields utils', () => { inline: { kind: 'absent' }, }) }) + + // Shorthand style fields. Measured against the Rust extractor, which + // resolves a same-file string constant behind the singular `styleUrl` + // but drops the array-valued forms — so the two need opposite answers, + // for the same reason the spread above stays absent: match what the + // compiled component actually ends up with. + it('reports a shorthand singular `styleUrl` as unreadable, not absent', () => { + const src = `@Component({ template: '

', styleUrl })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) + }) + + it('leaves a shorthand `styleUrls` absent, which the compiler also drops', () => { + const src = `@Component({ template: '

', styleUrls })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('leaves a shorthand inline `styles` absent, which the compiler also drops', () => { + const src = `@Component({ template: '

', styles })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.inline).toEqual({ kind: 'absent' }) + }) + + it('does not let an unrelated shorthand degrade a readable field', () => { + const src = `@Component({ selector, styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + // The trap in accepting a bare key: an identifier used as a VALUE is not + // a shorthand property, and reading it as one would fall back on a field + // that is right there and readable. + it('does not mistake a style-named identifier used as a value for a shorthand', () => { + const src = `@Component({ selector: styleUrl, styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('does not let an unrelated method degrade a readable field', () => { + const src = `@Component({ foo() { return 1 }, styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + // A method or accessor named like a style field is not a style field: + // the compiler reads no styles from it either. + it('leaves a method named like a style field absent', () => { + const src = `@Component({ styleUrls() { return ['./a.css'] } })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('leaves a getter named like a style field absent', () => { + const src = `@Component({ get styleUrl() { return './a.css' } })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('reads the real field past a setter named like a style field', () => { + const src = `@Component({ set styleUrl(v) {}, styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('does not read a style field nested in a deeper object', () => { + const src = `@Component({ data: { styleUrls: ['./deep.css'] } })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('reads a field followed by a trailing comma', () => { + const src = `@Component({ styleUrls: ['./a.css'], })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('reads a shorthand style field that closes the object', () => { + // No trailing comma — the key is bounded by `}` rather than `,`. + const src = `@Component({ styleUrl })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) + }) }) // ----------------------------------------------------------------- diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index 009340ea0..51e126ff8 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -2245,4 +2245,56 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).not.toContain('PS_CMT_OLD_MARKER') expect(body).not.toContain('PS_CMT_SIB_MARKER') }) + it('serves the own stylesheet of a class using a shorthand `styleUrl`', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const ownCssPath = join(appDir, 'ps-shorthand-own.component.css') + const sibCssPath = join(appDir, 'ps-shorthand-sib.component.css') + const shorthandPath = join(appDir, 'ps-shorthand.component.ts') + writeFileSync(ownCssPath, '.PS_SHORTHAND_OWN_MARKER { color: red; }') + writeFileSync(sibCssPath, '.PS_SHORTHAND_SIB_MARKER { color: red; }') + + // The shorthand key is a style field this scan cannot resolve, but the + // Rust extractor folds the constant behind it. Reading it as "declares + // no styles" would strip the component's CSS. + const source = ` + import { Component } from '@angular/core'; + const styleUrl = './ps-shorthand-own.component.css'; + @Component({ + selector: 'app-ps-shorthand', + template: '

shorthand

', + styleUrl, + }) + export class ShorthandComponent {} + @Component({ + selector: 'app-ps-shorthand-sib', + template: '

sib

', + styleUrls: ['./ps-shorthand-sib.component.css'], + }) + export class ShorthandSiblingComponent {} + ` + writeFileSync(shorthandPath, source) + await transformSource(plugin, source, shorthandPath) + + writeFileSync(ownCssPath, '.PS_SHORTHAND_OWN_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(ownCssPath), + [{ id: normalizePath(ownCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + expectDispatched(mockServer, `${shorthandPath}@ShorthandComponent`) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${shorthandPath}@ShorthandComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_SHORTHAND_OWN_MARKER') + // The sibling's stylesheet rides along, because this is the file-level + // fallback: it is the union for the file. That is the known limitation + // tracked in #456, asserted the same way as the singular-constant case + // above. The defect fixed here is the own stylesheet going MISSING. + }) }) diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index b1344aa9a..8a3871777 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -235,30 +235,62 @@ function findFieldInArgs( argsRange: [number, number], field: string, openerChars: string, + shorthandMeans: 'absent' | 'unreadable' = 'absent', ): FieldValue { const [openParen, closeParen] = argsRange const stack: Ctx[] = ['paren'] let i = openParen + 1 + // Keys sit at the start of the object and after each comma; `:` hands over + // to the value. Without this, a style-named identifier used as a VALUE + // (`selector: styleUrl`) would read as a shorthand property. + let atKey = true while (i < closeParen) { - // Check for a field-key match BEFORE advancing. The match is only - // valid at the @Component's immediate object-literal depth - // (`['paren', 'brace']`) — anything deeper is a nested literal that + // A key match is only valid at the @Component's immediate object-literal + // depth (`['paren', 'brace']`) — anything deeper is a nested literal that // isn't the component's metadata. - const afterKey = - stack.length === 2 && stack[1] === 'brace' ? matchFieldKeyAt(code, i, field, closeParen) : -1 - if (afterKey !== -1) { - let j = afterKey - while (j < closeParen && WS_RE.test(code[j])) j++ - if (code[j] === ':') { - j++ - while (j < closeParen && WS_RE.test(code[j])) j++ - if (j < closeParen && openerChars.includes(code[j])) { - const end = findClosingDelim(code, j) - if (end !== -1 && end < closeParen) return { kind: 'literal', range: [j, end] } + if (stack.length === 2 && stack[1] === 'brace') { + const ch = code[i] + if (WS_RE.test(ch)) { + i++ + continue + } + const afterComment = skipComment(code, i, closeParen) + if (afterComment !== -1) { + i = afterComment + continue + } + if (ch === ',') { + atKey = true + i++ + continue + } + if (ch === ':') { + atKey = false + i++ + continue + } + + const afterKey = atKey ? matchFieldKeyAt(code, i, field, closeParen) : -1 + if (afterKey !== -1) { + const j = skipToToken(code, afterKey, closeParen) + if (code[j] === ':') { + const v = skipToToken(code, j + 1, closeParen) + if (v < closeParen && openerChars.includes(code[v])) { + const end = findClosingDelim(code, v) + if (end !== -1 && end < closeParen) return { kind: 'literal', range: [v, end] } + } + // The key is here; its value is not a shape we can read. + return { kind: 'unreadable' } + } + // Shorthand (`{ styleUrl }`): the key stands alone, so the value is + // a binding this scan cannot follow. Whether that is unknowable or + // genuinely nothing depends on the field — see `locateStyleFieldsFor`. + // Anything else after the key (`(` for a method or accessor) is not + // this field at all, so keep scanning. + if ((code[j] === ',' || code[j] === '}') && shorthandMeans === 'unreadable') { + return { kind: 'unreadable' } } - // The key is here; its value is not a shape we can read. - return { kind: 'unreadable' } } } i = advanceOneToken(code, i, stack, closeParen) @@ -266,6 +298,21 @@ function findFieldInArgs( return { kind: 'absent' } } +/** Index of the next character at or after `i` that is not whitespace or a comment. */ +function skipToToken(code: string, i: number, end: number): number { + let j = i + while (j < end) { + if (WS_RE.test(code[j])) { + j++ + continue + } + const afterComment = skipComment(code, j, end) + if (afterComment === -1) break + j = afterComment + } + return j +} + function locateFieldInsideArgs( code: string, argsRange: [number, number], @@ -798,6 +845,14 @@ export interface ClassStyleFields { * `styleUrls` wins over `styleUrl` when a decorator carries both, unless the * plural form is absent; Angular itself rejects that combination, so this only * makes the choice deterministic. One decorator scan serves all three lookups. + * + * A shorthand property (`{ styleUrl }`) is unreadable for the singular form + * only. Measured against the Rust extractor: it folds a same-file string + * constant behind `styleUrl`, so calling that absent would strip real CSS — + * but it drops the array-valued `styleUrls` and `styles` shorthands, so the + * compiled component genuinely has no styles from them and falling back would + * hand the class its siblings' stylesheets. Same reasoning as the spread in + * `hasUnreadableKey`: match what the component actually compiles to. */ export function locateStyleFieldsFor(code: string, className: string): ClassStyleFields | null { const found = locateComponentDecorators(code).find((d) => d.className === className) @@ -813,7 +868,7 @@ export function locateStyleFieldsFor(code: string, className: string): ClassStyl return { urls: classify( plural.kind === 'absent' - ? findFieldInArgs(code, found.argsRange, 'styleUrl', TEMPLATE_OPENERS) + ? findFieldInArgs(code, found.argsRange, 'styleUrl', TEMPLATE_OPENERS, 'unreadable') : plural, ), inline: classify(findFieldInArgs(code, found.argsRange, 'styles', STYLES_OPENERS)), From 21d92d4896b9937bf4f51beef7ede8fc9fea8c1c Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 18:43:51 +0800 Subject: [PATCH 07/10] test(vite): pin comment placement in style field declarations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `styleUrls /* why */: ['./x.css']` was read as absent at c2342b0, so the endpoint served nothing while Rust resolved the stylesheet. Round 8's skipToToken fixed it incidentally while fixing shorthand properties, and nothing pinned it — an incidental fix can regress silently. The earlier 26-form audit framed itself around key SHAPES and never asked where a comment may sit inside a declaration. That axis is covered now: around the key and colon, before the key, the line-comment form, the singular form, inside the array before, between and after elements, after a quoted key's closing quote, two consecutive comments, and comments carrying decoy syntax or an apostrophe. A comment-only array stays parsed-empty, which is right — it is still a valid empty array. The shorthand terminator cases stay unreadable, so the guards pin round 8's behavior rather than quietly widening it. These are not vacuous: replayed against c2342b0, 7 fail — 5 as absent, the CSS-stripping outcome, and 2 by falling back instead of reading the value. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index 5b3be6dc9..2a47cf304 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -911,6 +911,131 @@ class Foo {}` const sRange = locateStylesFieldFor(src, 'Foo')! expect(src.slice(sRange[0], sRange[1] + 1)).toBe(`['real']`) }) + + // A comment may sit anywhere inside a style field declaration, and the + // Rust extractor reads straight past it. Every placement below was + // measured against `extractComponentUrls`, which reports `./x.css` for + // each one. Reading any of them as absent would tell the caller the + // class declares no styles, which strips the component's CSS. + describe('comment placement within a style field declaration', () => { + const urlsOf = (src: string) => { + const field = locateStyleFieldsFor(src, 'Foo')!.urls + expect(field.kind).toBe('literal') + return readStringLiterals(src, (field as { range: [number, number] }).range).literals + } + const decorator = (field: string) => + `@Component({ template: '

', ${field} })\nexport class Foo {}` + + it('reads a field with a block comment between the key and the colon', () => { + expect(urlsOf(decorator(`styleUrls /* why */: ['./x.css']`))).toEqual(['./x.css']) + }) + + it('reads a field with a line comment between the key and the colon', () => { + expect(urlsOf(decorator(`styleUrls // why\n: ['./x.css']`))).toEqual(['./x.css']) + }) + + it('reads a field with a comment between the colon and the value', () => { + expect(urlsOf(decorator(`styleUrls: /* why */ ['./x.css']`))).toEqual(['./x.css']) + }) + + it('reads a field with a comment before the key', () => { + expect(urlsOf(decorator(`/* why */ styleUrls: ['./x.css']`))).toEqual(['./x.css']) + }) + + it('reads a field with two comments between the key and the colon', () => { + expect(urlsOf(decorator(`styleUrls /* a */ /* b */: ['./x.css']`))).toEqual(['./x.css']) + }) + + it('reads a quoted key with a comment before the colon', () => { + expect(urlsOf(decorator(`'styleUrls' /* why */: ['./x.css']`))).toEqual(['./x.css']) + }) + + it('reads the singular `styleUrl` with a comment before the colon', () => { + expect(urlsOf(decorator(`styleUrl /* why */: './x.css'`))).toEqual(['./x.css']) + }) + + it('reads the singular `styleUrl` with a comment after the colon', () => { + expect(urlsOf(decorator(`styleUrl: /* why */ './x.css'`))).toEqual(['./x.css']) + }) + + it('reads an array whose first literal follows a comment', () => { + expect(urlsOf(decorator(`styleUrls: [/* why */ './x.css']`))).toEqual(['./x.css']) + }) + + it('reads an array whose first literal follows a line comment', () => { + expect(urlsOf(decorator(`styleUrls: [// why\n './x.css']`))).toEqual(['./x.css']) + }) + + it('reads an array with a comment before the separating comma', () => { + expect(urlsOf(decorator(`styleUrls: ['./x.css' /* why */, './y.css']`))).toEqual([ + './x.css', + './y.css', + ]) + }) + + it('reads an array with a comment after the separating comma', () => { + expect(urlsOf(decorator(`styleUrls: ['./x.css', /* why */ './y.css']`))).toEqual([ + './x.css', + './y.css', + ]) + }) + + it('reads an array with a comment after the last literal', () => { + expect(urlsOf(decorator(`styleUrls: ['./x.css' /* why */]`))).toEqual(['./x.css']) + }) + + it('reads an inline `styles` field with a comment before the colon', () => { + const src = decorator(`styles /* why */: ['.a{}']`) + const field = locateStyleFieldsFor(src, 'Foo')!.inline + expect(field.kind).toBe('literal') + expect( + readStringLiterals(src, (field as { range: [number, number] }).range).literals, + ).toEqual(['.a{}']) + }) + + // An array holding only a comment is still a syntactically valid empty + // array: a definite "no styles", not an unreadable value. + it('reads an array holding only a comment as definitively empty', () => { + const src = decorator(`styleUrls: [/* why */]`) + const field = locateStyleFieldsFor(src, 'Foo')!.urls + expect(field.kind).toBe('literal') + expect( + readStringLiterals(src, (field as { range: [number, number] }).range).literals, + ).toEqual([]) + }) + + // Comment *contents* must never be mistaken for structure. + it('ignores a colon, comma and brackets inside the comment', () => { + expect(urlsOf(decorator(`styleUrls /* a: b, c ] [ */: ['./x.css']`))).toEqual(['./x.css']) + }) + + it('ignores a decoy style field inside the comment', () => { + expect(urlsOf(decorator(`styleUrls /* styleUrl: './fake.css' */: ['./x.css']`))).toEqual([ + './x.css', + ]) + }) + + it('ignores an apostrophe inside a comment between the key and the colon', () => { + expect(urlsOf(decorator(`styleUrls /* don't */: ['./x.css']`))).toEqual(['./x.css']) + }) + + // A comment must not turn a shorthand back into "absent": the compiler + // resolves a same-file constant behind the singular `styleUrl` + // (verified: `./sh.css`), so this class must still reach the fallback. + it('keeps a shorthand `styleUrl` unreadable when a comment precedes the closing brace', () => { + const src = `const styleUrl = './sh.css'; +@Component({ template: '

', styleUrl /* why */ }) +export class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) + }) + + it('keeps a shorthand `styleUrl` unreadable when a comment precedes the comma', () => { + const src = `const styleUrl = './sh.css'; +@Component({ template: '

', styleUrl /* why */, selector: 'a' }) +export class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) + }) + }) }) // ----------------------------------------------------------------- From 20c13a0c58949bad606f9f756359eac045fcad01 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 18:55:41 +0800 Subject: [PATCH 08/10] fix(vite): decode escaped identifier keys in style field scans A decorator may spell a key with a unicode escape. `styleUrls` is `styleUrls`, and the Rust extractor reads it. The text scanner did not, so it reported the field absent and the endpoint served the wrong CSS. `readIdentifierKey` now decodes `\uHHHH` and `\u{...}` in a key position, the only two escapes JS permits in an identifier. It validates each decoded character against the identifier start/continue classes for its position. A malformed or illegal escape marks the key unreadable, not absent, so the endpoint falls back instead of guessing. The same decoding now applies to quoted keys, which were unreadable before. `'styleUrls'` matches the field exactly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 100 ++++++++++++++- .../test/hmr-hot-update.test.ts | 51 ++++++++ .../vite-plugin/utils/decorator-fields.ts | 121 +++++++++++++++--- 3 files changed, 251 insertions(+), 21 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index 2a47cf304..35a997c70 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -554,10 +554,11 @@ describe('decorator-fields utils', () => { }) }) - it('reports both fields unreadable for a quoted key carrying an escape', () => { - // `'style\u0055rls'` is `styleUrls` after decoding, which the Rust - // extractor does and this scan deliberately does not. - const src = `@Component({ 'style\\u0055rls': ['./a.css'] })\nexport class Foo {}` + it('reports both fields unreadable for a quoted key whose escape is malformed', () => { + // A decodable escape is resolved instead — see the `escaped keys` + // block. Only an escape this scan cannot decode leaves the field name + // unknown, and then "absent" would be a guess rather than an answer. + const src = `@Component({ 'style\\u00ZZrls': ['./a.css'] })\nexport class Foo {}` expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ urls: { kind: 'unreadable' }, inline: { kind: 'unreadable' }, @@ -655,6 +656,97 @@ describe('decorator-fields utils', () => { const src = `@Component({ styleUrl })\nexport class Foo {}` expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) }) + + // Escaped identifier keys. `style\u0055rls` IS `styleUrls` to the + // TypeScript parser, and the Rust extractor resolves it (verified: it + // reports `./x.css`). Reading it as absent told the caller the class + // declares no styles, which strips the component's CSS. Decoding gives + // an exact per-class answer, where falling back would serve the + // file-level union — every sibling's CSS along with this class's own. + describe('escaped keys', () => { + it('decodes a \\uHHHH escape in a bare key', () => { + const src = `@Component({ style\\u0055rls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('decodes a \\u{…} escape in a bare key', () => { + const src = `@Component({ style\\u{55}rls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('decodes an escape at the first character of a bare key', () => { + const src = `@Component({ \\u0073tyleUrls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('decodes an escaped singular `styleUrl` key', () => { + const src = `@Component({ style\\u0055rl: './solo.css' })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./solo.css']) + }) + + it('decodes an escaped inline `styles` key', () => { + const src = `@Component({ style\\u0073: ['.x{}'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.inline)).toEqual(['.x{}']) + }) + + it('decodes a quoted key carrying an escape', () => { + // A quoted key is a string literal, so it decodes by string rules. + const src = `@Component({ 'style\\u0055rls': ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + + it('treats an escaped shorthand singular `styleUrl` like the plain one', () => { + const src = `@Component({ template: '

', style\\u0055rl })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) + }) + + it('leaves an escaped shorthand `styleUrls` absent, as the compiler drops it', () => { + const src = `@Component({ template: '

', style\\u0055rls })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('keeps the cross-match guards for decoded keys', () => { + const urls = `@Component({ style\\u0055rls: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(urls, 'Foo')!.inline).toEqual({ kind: 'absent' }) + const singular = `@Component({ style\\u0055rl: './a.css' })\nexport class Foo {}` + expect(locateStyleFieldsFor(singular, 'Foo')!.inline).toEqual({ kind: 'absent' }) + const inline = `@Component({ style\\u0073: ['.x{}'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(inline, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('does not match a decoded key that names something else', () => { + const src = `@Component({ style\\u0055rlsExtra: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('does not match a lookalike built from a non-ASCII letter', () => { + // Cyrillic \u0435 in place of `e` — a different identifier, and the + // compiler reports no styleUrls for it either. + const src = `@Component({ styl\u0435Urls: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'absent' }) + }) + + it('reports a malformed escape in a key as unreadable, not absent', () => { + const src = `@Component({ style\\u00ZZrls: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ + urls: { kind: 'unreadable' }, + inline: { kind: 'unreadable' }, + }) + }) + + it('reports a \\xHH escape in a key as unreadable — illegal in an identifier', () => { + const src = `@Component({ style\\x55rls: ['./a.css'] })\nexport class Foo {}` + expect(locateStyleFieldsFor(src, 'Foo')).toEqual({ + urls: { kind: 'unreadable' }, + inline: { kind: 'unreadable' }, + }) + }) + + it('does not let an escaped unrelated key degrade a readable field', () => { + const src = `@Component({ sel\\u0065ctor: 'a', styleUrls: ['./a.css'] })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + }) }) // ----------------------------------------------------------------- diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index 51e126ff8..28a7a7ec5 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -1805,6 +1805,57 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).toContain('PS_SINGCONST_OWN_MARKER') }) + it('serves the own stylesheet of a class whose `styleUrls` key is escaped', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const ownCssPath = join(appDir, 'ps-esckey-own.component.css') + const sibCssPath = join(appDir, 'ps-esckey-sib.component.css') + const escKeyPath = join(appDir, 'ps-esckey.component.ts') + writeFileSync(ownCssPath, '.PS_ESCKEY_OWN_MARKER { color: red; }') + writeFileSync(sibCssPath, '.PS_ESCKEY_SIB_MARKER { color: red; }') + + // `style\u0055rls` IS `styleUrls` to the TypeScript parser, and the Rust + // extractor resolves it. Reading the key as absent said "this class has + // no styles" and served none, stripping the component's CSS. + const source = ` + import { Component } from '@angular/core'; + @Component({ + selector: 'app-ps-esckey', + template: '

esc

', + style\\u0055rls: ['./ps-esckey-own.component.css'], + }) + export class EscKeyComponent {} + @Component({ + selector: 'app-ps-esckey-sib', + template: '

sib

', + styleUrls: ['./ps-esckey-sib.component.css'], + }) + export class EscKeySiblingComponent {} + ` + writeFileSync(escKeyPath, source) + await transformSource(plugin, source, escKeyPath) + + writeFileSync(ownCssPath, '.PS_ESCKEY_OWN_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(ownCssPath), + [{ id: normalizePath(ownCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + expectDispatched(mockServer, `${escKeyPath}@EscKeyComponent`) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${escKeyPath}@EscKeyComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_ESCKEY_OWN_MARKER') + // Decoding the key gives an exact per-class answer, so unlike the + // constant-valued fallback above, the sibling's CSS does not ride along. + expect(body).not.toContain('PS_ESCKEY_SIB_MARKER') + }) + it('serves no styles for an explicitly empty `styleUrls` array', async () => { const plugin = getAngularPlugin() const mockServer = await setupPluginWithRealConfig(plugin) diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index 8a3871777..bf128f696 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -325,22 +325,103 @@ function locateFieldInsideArgs( /** * If a key for `field` starts at `position`, return the index just past it; - * otherwise -1. Accepts the bare form (`styleUrls:`) and the quoted forms - * (`'styleUrls':`, `"styleUrls":`), which are valid TS and which the Rust - * extractor resolves. + * otherwise -1. Accepts the bare form (`styleUrls:`), the quoted forms + * (`'styleUrls':`, `"styleUrls":`), and either form written with escapes + * (`style\u0055rls`, `'style\u0055rls'`) — all of which are valid TS naming + * the same field, and all of which the Rust extractor resolves. * - * A quoted key must match `field` character for character. One written with - * an escape (`'style\u0055rls'`) is deliberately not decoded here — see - * `hasUnreadableKey`, which reports it as beyond this scan rather than - * letting it read as a key that isn't there. + * Escapes are decoded rather than refused. Refusing them would only be safe + * in the sense of falling back, and the fallback serves the file-level style + * union — every sibling's CSS along with this class's own. Decoding keeps the + * per-class answer exact. A key whose escapes cannot be decoded is not + * matched here; `hasUnreadableKey` reports it as beyond this scan. + * + * Decoding does not loosen the match: the decoded name must still equal + * `field` exactly, so `styleUrl` and `styleUrls` stay distinct. */ function matchFieldKeyAt(code: string, position: number, field: string, limit: number): number { if (isFieldKeyAt(code, position, field, limit)) return position + field.length const quote = code[position] - if (quote !== "'" && quote !== '"') return -1 - const close = findClosingDelim(code, position) - if (close === -1 || close >= limit) return -1 - return code.slice(position + 1, close) === field ? close + 1 : -1 + if (quote === "'" || quote === '"') { + const close = findClosingDelim(code, position) + if (close === -1 || close >= limit) return -1 + // A quoted key is a string literal, so it decodes by string rules. + return decodeEscapes(code.slice(position + 1, close)) === field ? close + 1 : -1 + } + const id = readIdentifierKey(code, position, limit) + return id !== null && id.name === field ? id.end : -1 +} + +/** + * Read an identifier starting at `position`, decoding escapes, and return the + * decoded name with the index just past it. Returns null when nothing + * identifier-like starts there. + * + * `name` is null when the identifier carries an escape this scan cannot + * resolve — malformed hex, a truncated `\u`, a decoded character not valid at + * its position, or any escape other than `\uHHHH` / `\u{…}`, which are the + * only two JavaScript permits inside an identifier. The caller must treat + * that as unknown rather than as a key that isn't there: the Rust extractor + * parses what this cannot, so "absent" would strip a component's real CSS. + */ +function readIdentifierKey( + code: string, + position: number, + limit: number, +): { name: string | null; end: number } | null { + let i = position + let name = '' + let first = true + let malformed = false + + while (i < limit) { + const ch = code[i] + + if (ch === '\\') { + if (code[i + 1] !== 'u') { + // `\x41`, `\n`, … are string escapes; in an identifier they are a + // syntax error, so the name cannot be known from the text. + malformed = true + i += 2 + first = false + continue + } + let codePoint = -1 + let next: number + if (code[i + 2] === '{') { + const close = code.indexOf('}', i + 3) + if (close === -1 || close >= limit) return { name: null, end: i + 2 } + const hex = code.slice(i + 3, close) + if (HEX_ONLY_RE.test(hex)) codePoint = parseInt(hex, 16) + next = close + 1 + } else { + const hex = code.slice(i + 2, i + 6) + if (hex.length === 4 && HEX_ONLY_RE.test(hex)) codePoint = parseInt(hex, 16) + next = i + 6 + } + if (codePoint < 0 || codePoint > 0x10ffff) { + malformed = true + } else { + const decoded = String.fromCodePoint(codePoint) + if (!(first ? IDENT_START_RE : IDENT_CONT_RE).test(decoded)) malformed = true + name += decoded + } + i = next + first = false + continue + } + + if ((first ? IDENT_START_RE : IDENT_CONT_RE).test(ch)) { + name += ch + i++ + first = false + continue + } + break + } + + if (i === position) return null + return { name: malformed ? null : name, end: i } } /** @@ -879,11 +960,11 @@ export function locateStyleFieldsFor(code: string, className: string): ClassStyl * Whether the decorator's own object literal holds a key this scan cannot * resolve to a name, which makes "the class declares no styles" unknowable. * - * Two forms qualify, both of which the Rust extractor DOES resolve, so - * reading them as absent would strip a component's CSS: - * - a computed key (`[K]: [...]`); - * - a quoted key carrying an escape (`'style\u0055rls'`), which this scan - * matches literally and so would miss. + * Two forms qualify, so reading them as absent would strip a component's CSS: + * - a computed key (`[K]: [...]`), which the Rust extractor DOES resolve; + * - a key whose escapes cannot be decoded — a malformed `\u`, or an escape + * that is illegal in an identifier. A key whose escapes DO decode is not + * unreadable: `matchFieldKeyAt` resolves it to a name exactly. * * A spread (`...BASE`) deliberately does NOT qualify. The Rust extractor * drops it too, so the compiled component genuinely has no styles from it; @@ -927,11 +1008,17 @@ function hasUnreadableKey(code: string, argsRange: [number, number]): boolean { if (ch === "'" || ch === '"') { const close = findClosingDelim(code, i) if (close === -1 || close >= closeParen) return true - if (code.slice(i + 1, close).includes('\\')) return true + if (decodeEscapes(code.slice(i + 1, close)) === null) return true atKey = false i = close + 1 continue } + const id = readIdentifierKey(code, i, closeParen) + if (id !== null) { + if (id.name === null) return true + i = id.end + continue + } } } i = advanceOneToken(code, i, stack, closeParen) From 7f483d08d42721e559ecba938b30a1ff7b41edfc Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 24 Aug 2026 19:12:21 +0800 Subject: [PATCH 09/10] fix(vite): reject a style literal that only starts a larger expression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `styleUrl: './a.css' + SUFFIX` opens with a literal the value does not denote. The scan returned that leading piece and called the field readable, so the endpoint served a stylesheet the compiled component never had — and, being a confident answer, skipped the fallback that exists for values the scan cannot resolve. A literal now counts as the value only when the property ends at its closing delimiter. `endsPropertyValue` skips whitespace and comments, then requires `,`, `}` or `)`. Anything else marks the field unreadable. Measured against the Rust extractor, which compiles no styles for any of these: concatenation, a method call, `as`, `as const`, `!`, `satisfies`. The guard is shared with `template` and `templateUrl`, which moves those from wrong to correct the same way. `stripComponentMetadata` strips one less shape, which only downgrades HMR to a full reload. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 86 +++++++++++++++++++ .../vite-plugin/utils/decorator-fields.ts | 29 ++++++- 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index 35a997c70..67c84922a 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -657,6 +657,92 @@ describe('decorator-fields utils', () => { expect(locateStyleFieldsFor(src, 'Foo')!.urls).toEqual({ kind: 'unreadable' }) }) + // A value that merely STARTS with a literal does not denote it. Every + // form below was measured against the Rust extractor: it reports no + // styleUrls and compiles no styles for any of them. Returning the + // leading literal would hand the endpoint a stylesheet the component + // never had, and — being a confident answer — would skip the fallback + // that exists for exactly this case. + describe('a literal that is only the start of a larger expression', () => { + const urls = (field: string) => + locateStyleFieldsFor( + `const SUFFIX = '.s.css';\nconst MORE: string[] = ['./m.css'];\n@Component({ template: '

', ${field} })\nexport class Foo {}`, + 'Foo', + )!.urls + const inline = (field: string) => + locateStyleFieldsFor( + `const EXTRA = '.b{}';\n@Component({ template: '

', ${field} })\nexport class Foo {}`, + 'Foo', + )!.inline + + it('rejects a singular `styleUrl` concatenated with an identifier', () => { + expect(urls(`styleUrl: './a.css' + SUFFIX`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a singular `styleUrl` concatenated with another literal', () => { + expect(urls(`styleUrl: './a.css' + './b.css'`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a `styleUrls` array concatenated with an identifier', () => { + expect(urls(`styleUrls: './a.css' + SUFFIX`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects an inline `styles` value concatenated with an identifier', () => { + expect(inline(`styles: '.a{}' + EXTRA`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a method call on a style literal', () => { + expect(urls(`styleUrl: './a.css'.replace('a', 'b')`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a method call on a `styleUrls` array literal', () => { + expect(urls(`styleUrls: ['./a.css'].concat(MORE)`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a method call on an inline `styles` array literal', () => { + expect(inline(`styles: ['.a{}'].concat(MORE)`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a TypeScript `as` assertion after a style literal', () => { + expect(urls(`styleUrl: './a.css' as string`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a TypeScript `as const` assertion after a `styleUrls` array', () => { + expect(urls(`styleUrls: ['./a.css'] as const`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a TypeScript `as` assertion after an inline `styles` array', () => { + expect(inline(`styles: ['.a{}'] as string[]`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a non-null assertion after a style literal', () => { + expect(urls(`styleUrl: './a.css'!`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a `satisfies` clause after a style literal', () => { + expect(urls(`styleUrl: './a.css' satisfies string`)).toEqual({ kind: 'unreadable' }) + }) + + it('rejects a trailing expression hidden behind a comment', () => { + expect(urls(`styleUrl: './a.css' /* why */ + SUFFIX`)).toEqual({ kind: 'unreadable' }) + }) + + // The other side of the rule: a value the property really does end at + // stays readable, whether a comma, the object's brace or a comment + // closes it out. + it.each([ + [`styleUrl: './a.css'`, `the object's closing brace`], + [`styleUrl: './a.css',`, 'a trailing comma'], + [`styleUrl: './a.css' /* why */`, 'a block comment then the brace'], + [`styleUrl: './a.css' // why\n`, 'a line comment then the brace'], + [`styleUrls: ['./a.css']`, 'an array at the closing brace'], + [`styleUrls: ['./a.css'],`, 'an array with a trailing comma'], + ])('still reads %j, ended by %s', (field) => { + const src = `@Component({ template: '

', ${field} })\nexport class Foo {}` + expect(literalsIn(src, locateStyleFieldsFor(src, 'Foo')!.urls)).toEqual(['./a.css']) + }) + }) + // Escaped identifier keys. `style\u0055rls` IS `styleUrls` to the // TypeScript parser, and the Rust extractor resolves it (verified: it // reports `./x.css`). Reading it as absent told the caller the class diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index bf128f696..b1a8cbaf9 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -278,7 +278,16 @@ function findFieldInArgs( const v = skipToToken(code, j + 1, closeParen) if (v < closeParen && openerChars.includes(code[v])) { const end = findClosingDelim(code, v) - if (end !== -1 && end < closeParen) return { kind: 'literal', range: [v, end] } + // The literal is the value only when the property ENDS at its + // closing delimiter. `'./a.css' + SUFFIX` opens with a literal + // it does not denote, and so do `['./a.css'].concat(MORE)` and + // `'./a.css' as const`. Returning that leading piece would name + // a stylesheet the compiled component never had — and, being a + // confident answer, would skip the fallback that exists for + // values this scan cannot resolve. + if (end !== -1 && end < closeParen && endsPropertyValue(code, end + 1, closeParen)) { + return { kind: 'literal', range: [v, end] } + } } // The key is here; its value is not a shape we can read. return { kind: 'unreadable' } @@ -298,6 +307,24 @@ function findFieldInArgs( return { kind: 'absent' } } +/** + * Whether a property value ends at `i`. Inside an object literal a value is + * closed by `,` or the object's own `}` — or, defensively, the decorator's + * `)`. Whitespace and comments are skipped first, so a comment may sit + * between the value and its terminator. + * + * Anything else at that position means what was just read is only the LEADING + * part of a larger expression — a concatenation, a call, a TypeScript + * assertion — and so is not the value at all. Running out of text counts as + * ending, since nothing is left to extend the value with. + */ +function endsPropertyValue(code: string, i: number, end: number): boolean { + const j = skipToToken(code, i, end) + if (j >= end) return true + const ch = code[j] + return ch === ',' || ch === '}' || ch === ')' +} + /** Index of the next character at or after `i` that is not whitespace or a comment. */ function skipToToken(code: string, i: number, end: number): number { let j = i From 39b4d450f6fe6718ccde7bbffe8e456979ce4240 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 25 Aug 2026 09:54:50 +0800 Subject: [PATCH 10/10] fix(vite): skip a spread element when reading a style array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `styleUrls: [...SHARED, './own.css']` marked the array incomplete, so `extractClassStylesFor` returned null and the endpoint fell back to the file-level union — handing the class every sibling stylesheet in the file. The deciding property is structural. `extract_string_array` asks each element for `as_expression()`, and `ArrayExpressionElement::is_expression()` enumerates neither `SpreadElement` nor `Elision`. A spread is therefore dropped before any value resolution, for every possible program, with the const table never consulted. An element that IS an expression reaches the resolver, which folds constants this text scan cannot read — so those still mark the array unknown, and the guard added in 7f483d0 stands. The spread is skipped whole via `advanceOneToken`, so a literal nested inside it is never mistaken for one of this class's styles; an unclosed delimiter still reports unknown rather than guessing. This also settles a contradiction inside the file: `hasUnreadableKey` already treats a decorator-level `...BASE` as droppable, on exactly this reasoning. Measured against the Rust extractor across 12 array shapes: every answer the scan now asserts matches, and `[S, './own.css']` still bails because Rust folds the constant. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01665HouXD8imyMphWe1k7Dx --- .../test/decorator-fields.test.ts | 190 +++++++++++++++++- .../test/hmr-hot-update.test.ts | 51 +++++ .../vite-plugin/utils/decorator-fields.ts | 50 ++++- 3 files changed, 283 insertions(+), 8 deletions(-) diff --git a/napi/angular-compiler/test/decorator-fields.test.ts b/napi/angular-compiler/test/decorator-fields.test.ts index 67c84922a..816d14c9a 100644 --- a/napi/angular-compiler/test/decorator-fields.test.ts +++ b/napi/angular-compiler/test/decorator-fields.test.ts @@ -948,8 +948,194 @@ describe('decorator-fields utils', () => { expect(readOf(`[SOME_CONST, './a.css']`).complete).toBe(false) }) - it('reports a spread entry as incomplete', () => { - expect(readOf(`[...SHARED, './a.css']`).complete).toBe(false) + // ----------------------------------------------------------------- + // Spread elements (`...X`) and elisions (holes). + // + // `extract_string_array` asks every array element for `as_expression()`, + // and `ArrayExpressionElement::is_expression()` enumerates neither + // `SpreadElement` nor `Elision`. Both are therefore dropped BEFORE the + // value resolver runs, for every possible program — the const table is + // never consulted for them. Dropping them here keeps the array complete + // and matches what the component actually compiles to; calling it + // unknown would hand the class its siblings' stylesheets instead. + // + // Measured against the Rust extractor: + // [...SHARED, './own.css'] -> ["./own.css"] + // [...SHARED] -> [] + // [S, './own.css'] (S a const) -> ["./shared.css", "./own.css"] + // + // Every OTHER element IS an expression the resolver may fold from + // constants this scan cannot read, so those still mark the array + // unknown. Same reasoning as the decorator-level spread in + // `hasUnreadableKey`; the two now agree. + // ----------------------------------------------------------------- + it('drops a leading spread and reads the rest, like the compiler', () => { + expect(readOf(`[...SHARED, './a.css']`)).toEqual({ + literals: ['./a.css'], + complete: true, + }) + }) + + it('drops a trailing spread and reads the rest', () => { + expect(readOf(`['./a.css', ...SHARED]`)).toEqual({ + literals: ['./a.css'], + complete: true, + }) + }) + + it('reads a spread-only array as declaring nothing', () => { + expect(readOf(`[...SHARED]`)).toEqual({ literals: [], complete: true }) + }) + + it('reads an array of two spreads as declaring nothing', () => { + expect(readOf(`[...S1, ...S2]`)).toEqual({ literals: [], complete: true }) + }) + + it('drops a spread of an inline array literal', () => { + // The compiler drops the element whole, so a literal INSIDE the spread + // is not part of this component's styles. + expect(readOf(`[...['./shared.css'], './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('never leaks a literal used as a computed member inside a spread', () => { + expect(readOf(`[...obj['./leak.css'], './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('never leaks a literal passed as a call argument inside a spread', () => { + expect(readOf(`[...f('./leak.css'), './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('never leaks a literal inside an object literal in a spread', () => { + expect(readOf(`[...Object.values({ k: './leak.css' }), './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('keeps a comma inside parens from ending the spread element', () => { + expect(readOf(`[...f(a, b), './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('keeps a comma inside a nested array from ending the spread element', () => { + expect(readOf(`[...['./x.css', './y.css'], './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('ignores a block comment inside a spread', () => { + expect(readOf(`[.../* , './leak.css' */ SHARED, './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('ignores an apostrophe and a comma inside a line comment in a spread', () => { + expect(readOf(`[...SHARED // don't , './leak.css'\n, './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('skips an interpolated template literal inside a spread', () => { + // The `${...}` pushes a brace context, so the comma inside the template + // does not end the element. + expect(readOf("[...tag`${DIR}/a, b`, './own.css']")).toEqual({ + literals: ['./own.css'], + complete: true, + }) + }) + + it('drops a spread in a `styleUrls` array too', () => { + const src = `@Component({ styleUrls: [...SHARED, './a.css'] })\nclass Foo {}` + expect(readStringLiterals(src, locateStyleUrlsFor(src, 'Foo')!)).toEqual({ + literals: ['./a.css'], + complete: true, + }) + }) + + it('drops a spread in an inline `styles` array', () => { + expect(readOf(`[...SHARED_INLINE, ':host{}']`)).toEqual({ + literals: [':host{}'], + complete: true, + }) + }) + + it('reports an unterminated string inside a spread as incomplete', () => { + // The quote never closes, so where the element ends is unknowable. + // A real file cannot produce this range (the locator needs a balanced + // `]`), but the scan must refuse to guess rather than run off. + const src = `[...f('./leak.css)]` + expect(readStringLiterals(src, [0, src.length - 1])).toEqual({ + literals: [], + complete: false, + }) + }) + + it('reports an unclosed paren inside a spread as incomplete', () => { + const src = `[...f(a]` + expect(readStringLiterals(src, [0, src.length - 1])).toEqual({ + literals: [], + complete: false, + }) + }) + + it('reports a spread ending in an escape as incomplete', () => { + // The trailing escape consumes the array's own `]`, so the scan + // overshoots the range: incomplete, not a guess. + const src = String.raw`[...'a\]` + expect(readStringLiterals(src, [0, src.length - 1])).toEqual({ + literals: [], + complete: false, + }) + }) + + it('does not treat a leading decimal point as a spread', () => { + expect(readOf(`[.5, './a.css']`).complete).toBe(false) + }) + + it('does not treat two dots as a spread', () => { + expect(readOf(`[..X, './a.css']`).complete).toBe(false) + }) + + it('does not treat a member access as a spread', () => { + expect(readOf(`[STYLES.a, './a.css']`).complete).toBe(false) + }) + + it('does not treat an optional chain as a spread', () => { + expect(readOf(`[STYLES?.a, './a.css']`).complete).toBe(false) + }) + + it('still reports a bare identifier element as incomplete, unlike a spread', () => { + // Measured: the Rust extractor DOES fold `S` from the const table, so + // the literals gathered here are not the component's full style list. + expect(readOf(`[S, './own.css']`)).toEqual({ + literals: ['./own.css'], + complete: false, + }) + }) + + it('reads across a leading elision, which the compiler also drops', () => { + expect(readOf(`[, './a.css']`)).toEqual({ literals: ['./a.css'], complete: true }) + }) + + it('reads across an elision between two entries', () => { + expect(readOf(`['./a.css', , './b.css']`)).toEqual({ + literals: ['./a.css', './b.css'], + complete: true, + }) }) it('reports an interpolated template literal as incomplete', () => { diff --git a/napi/angular-compiler/test/hmr-hot-update.test.ts b/napi/angular-compiler/test/hmr-hot-update.test.ts index 28a7a7ec5..b95808c2b 100644 --- a/napi/angular-compiler/test/hmr-hot-update.test.ts +++ b/napi/angular-compiler/test/hmr-hot-update.test.ts @@ -2201,6 +2201,57 @@ describe('@ng/component endpoint resolves the styles per class', () => { expect(body).not.toBe('') expect(body).not.toContain('PS_SPREAD_SIB_MARKER') }) + + // A spread INSIDE a `styleUrls` array is dropped by the compiler the same + // way, before any constant is resolved — so this class compiles to exactly + // the one path it spells out. Reading the array as unknown would fall back + // to the file-level list and serve it its sibling's CSS as well. + it('serves only the spelled-out entry of a styleUrls array with a spread', async () => { + const plugin = getAngularPlugin() + const mockServer = await setupPluginWithRealConfig(plugin) + + const ownCssPath = join(appDir, 'ps-spreadarr-own.component.css') + const sibCssPath = join(appDir, 'ps-spreadarr-sib.component.css') + const spreadArrPath = join(appDir, 'ps-spreadarr.component.ts') + writeFileSync(ownCssPath, '.PS_SPREADARR_OWN_MARKER { color: red; }') + writeFileSync(sibCssPath, '.PS_SPREADARR_SIB_MARKER { color: blue; }') + + const source = ` + import { Component } from '@angular/core'; + const SHARED = []; + @Component({ + selector: 'app-ps-spreadarr', + template: '

own

', + styleUrls: [...SHARED, './ps-spreadarr-own.component.css'], + }) + export class SpreadArrayComponent {} + @Component({ + selector: 'app-ps-spreadarr-sib', + template: '

sib

', + styleUrls: ['./ps-spreadarr-sib.component.css'], + }) + export class SpreadArraySiblingComponent {} + ` + writeFileSync(spreadArrPath, source) + await transformSource(plugin, source, spreadArrPath) + + // Edit the SIBLING's stylesheet; the fan-out queues both classes. + writeFileSync(sibCssPath, '.PS_SPREADARR_SIB_MARKER { color: green; }') + const ctx = createMockHmrContext( + normalizePath(sibCssPath), + [{ id: normalizePath(sibCssPath) }], + mockServer, + ) + await callHandleHotUpdate(plugin, ctx) + + const body = await invokeAngularMiddleware( + getMiddleware(mockServer), + `${spreadArrPath}@SpreadArrayComponent`, + ) + expect(body).not.toBe('') + expect(body).toContain('PS_SPREADARR_OWN_MARKER') + expect(body).not.toContain('PS_SPREADARR_SIB_MARKER') + }) it('serves a styleUrls entry written with a JavaScript escape', async () => { const plugin = getAngularPlugin() const mockServer = await setupPluginWithRealConfig(plugin) diff --git a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts index b1a8cbaf9..ddea7b3fd 100644 --- a/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts +++ b/napi/angular-compiler/vite-plugin/utils/decorator-fields.ts @@ -850,10 +850,14 @@ function decodeEscapes(raw: string): string | null { * The literals read out of a field value, and whether every element was read. * * `complete: false` means the value holds something this scan cannot resolve — - * an identifier, a spread, an interpolated template literal, an unterminated - * string. The literals gathered alongside it are NOT a partial answer to act - * on: a caller that used them would silently drop the stylesheets the - * unreadable elements stand for. Treat an incomplete read as unknown. + * an identifier, an interpolated template literal, an unterminated string. + * The literals gathered alongside it are NOT a partial answer to act on: a + * caller that used them would silently drop the stylesheets the unreadable + * elements stand for. Treat an incomplete read as unknown. + * + * A spread element (`...X`) and an elision are the exceptions: the Rust + * extractor drops both before resolving anything, so skipping them leaves + * the read complete. * * An empty array is the opposite case: `complete: true` with no literals is a * definitive "this field declares nothing". @@ -874,7 +878,9 @@ export interface StringLiteralsRead { * * Inside the array body, whitespace, commas and comments are skipped, and * each literal is delimited with `findClosingDelim`, so escape sequences and - * apostrophes inside comments cannot be mistaken for delimiters. + * apostrophes inside comments cannot be mistaken for delimiters. A spread + * element is skipped whole — see the branch below for why that keeps the + * read complete. * * Returns the raw inner contents — no unescaping, no trimming — because HMR * delivers these verbatim. @@ -922,7 +928,39 @@ export function readStringLiterals(code: string, range: [number, number]): Strin i = close + 1 continue } - // An identifier, spread, call or nested array: this element cannot be + if (ch === '.' && code[i + 1] === '.' && code[i + 2] === '.') { + // A spread element (`...X`). The Rust extractor asks each element for + // `as_expression()`, and `ArrayExpressionElement::is_expression()` + // enumerates neither `SpreadElement` nor `Elision` — so a spread is + // dropped BEFORE any value resolution, for every possible program, + // with the const table never consulted. Dropping it here too leaves + // the array complete and matches what the class compiles to; calling + // it unknown would hand the class its siblings' stylesheets instead. + // (An elision needs no branch: the comma skip above already reads + // past a hole.) Same reasoning as the spread in `hasUnreadableKey`. + // + // The whole element is skipped, so a literal nested inside it is + // never mistaken for one of this class's styles. `advanceOneToken` + // does the delimiter tracking, so nesting, strings, template + // interpolations and comments all hold a comma harmlessly. + const spreadStack: Ctx[] = [] + let j = i + 3 + while (j < end) { + if (spreadStack.length === 0 && code[j] === ',') break + j = advanceOneToken(code, j, spreadStack, end) + } + // Ran past the array's closer, or ended inside an unclosed string, + // comment or bracket: where this element stops is unknowable, so + // report unknown rather than guess. `j > i`, so `i` still advances. + if (j > end || spreadStack.length > 0) { + complete = false + break + } + i = j + continue + } + // An identifier, call or nested array: this element IS an expression, + // which the Rust resolver may fold from constants this scan cannot // read, so the array as a whole is unknown. Every branch above advances // `i`, so the scan terminates. complete = false