diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c6cd642ad..730d0ee811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -147,7 +147,7 @@ Breaking changes in this release: - New debug API, by [@compulim](https://github.com/compulim) in PR [#5663](https://github.com/microsoft/BotFramework-WebChat/pull/5663) and PR [#5664](https://github.com/microsoft/BotFramework-WebChat/pull/5664), see [`DEBUGGING.md`](docs/DEBUGGING.md) for more - Debug into element: open F12, select the subject in Element pane, type `$0.webChat.debugger` - Breakpoint: open F12, select the subject in Element pane, type `$0.webChat.breakpoint.incomingActivity` -- The `botframework-webchat` package now uses CSS modules for styling purposes, in PR [#5666](https://github.com/microsoft/BotFramework-WebChat/pull/5666), by [@OEvgeny](https://github.com/OEvgeny) +- The `botframework-webchat` package now uses CSS modules for styling purposes, in PR [#5666](https://github.com/microsoft/BotFramework-WebChat/pull/5666), in PR [#5677](https://github.com/microsoft/BotFramework-WebChat/pull/5677) by [@OEvgeny](https://github.com/OEvgeny) - 👷🏻 Added `npm run build-browser` script for building test harness package only, in PR [#5667](https://github.com/microsoft/BotFramework-WebChat/pull/5667), by [@compulim](https://github.com/compulim) ### Changed diff --git a/packages/api/package.json b/packages/api/package.json index c302546fc0..6e6fea218c 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -78,7 +78,7 @@ "build": "npm run --if-present build:pre && npm run build:run && npm run --if-present build:post", "build:post": "npm run build:post:dtsroll && npm run build:post:validate:dts", "build:post:dtsroll": "dtsroll ./dist/*.d.*", - "build:post:validate:dts": "if grep -q -P '@msinternal\\/' dist/*.d.* 2>/dev/null; then echo \"Error: dist/*.d.* is not compiled by dtsroll\" >&2; exit 1; fi", + "build:post:validate:dts": "vg ast-check dist-types ./dist/*.d.*", "build:pre": "npm run build:pre:local-dependencies && npm run build:pre:watch && npm run build:pre:globalize", "build:pre:globalize": "node scripts/createPrecompiledGlobalize.mjs", "build:pre:local-dependencies": "../../scripts/npm/build-local-dependencies.sh", diff --git a/packages/bundle/esbuild.static.mjs b/packages/bundle/esbuild.static.mjs index 235a55d6e7..863974e53e 100644 --- a/packages/bundle/esbuild.static.mjs +++ b/packages/bundle/esbuild.static.mjs @@ -83,7 +83,7 @@ const BASE_CONFIG = { plugins: [ cssPlugin, injectCSSPlugin({ - getCSSText: (_source, cssFiles) => cssFiles.find(({ path }) => path.endsWith('botframework-webchat.css'))?.text, + ignoreCSSEntries: ['static/botframework-webchat/component.css'], stylesPlaceholder: bundleStyleContentPlaceholder }), { diff --git a/packages/bundle/package.json b/packages/bundle/package.json index 4d0ff7c2a5..5022e2ce4a 100644 --- a/packages/bundle/package.json +++ b/packages/bundle/package.json @@ -100,9 +100,12 @@ }, "scripts": { "build": "npm run --if-present build:pre && npm run build:run && npm run --if-present build:post", - "build:post": "npm run build:post:dtsroll && npm run build:post:validate:dts", + "build:post": "npm run build:post:dtsroll && npm run build:post:validate", "build:post:dtsroll": "dtsroll ./dist/*.d.*", - "build:post:validate:dts": "if grep -q -P '@msinternal\\/' dist/*.d.* 2>/dev/null; then echo \"Error: dist/*.d.* is not compiled by dtsroll\" >&2; exit 1; fi", + "build:post:validate": "npm run build:post:validate:css && npm run build:post:validate:inject-css && npm run build:post:validate:dts", + "build:post:validate:css": "vg ast-check lightning-css ./dist/*.css", + "build:post:validate:dts": "vg ast-check dist-types ./dist/*.d.*", + "build:post:validate:inject-css": "vg ast-check css-inject dist/*.js dist/*.mjs static/*.js", "build:pre": "npm run build:pre:local-dependencies && npm run build:pre:watch", "build:pre:local-dependencies": "../../scripts/npm/build-local-dependencies.sh", "build:pre:watch": "../../scripts/npm/build-watch.sh", diff --git a/packages/bundle/tsup.config.ts b/packages/bundle/tsup.config.ts index 58bff54be9..f10dedd7dc 100644 --- a/packages/bundle/tsup.config.ts +++ b/packages/bundle/tsup.config.ts @@ -47,6 +47,13 @@ const commonConfig = applyConfig(config => ({ // The way `microsoft-cognitiveservices-speech-sdk` imported the `uuid` package (in their `Guid.js`) is causing esbuild/tsup to proxy require() into __require() for dynamic loading. // Webpack 4 cannot statically analyze the code and failed with error "Critical dependency: require function is used in a way in which dependencies cannot be statically extracted". 'uuid' + ], + esbuildPlugins: [ + ...(config.esbuildPlugins ?? []), + injectCSSPlugin({ + ignoreCSSEntries: ['dist/botframework-webchat.component.css'], + stylesPlaceholder: bundleStyleContentPlaceholder, + }) ] })); @@ -64,11 +71,7 @@ export default defineConfig([ 'webchat-es5': './src/boot/iife/webchat-es5.ts', 'webchat-minimal': './src/boot/iife/webchat-minimal.ts' }, - esbuildPlugins: [ - ...(commonConfig.esbuildPlugins ?? []), - injectCSSPlugin({ stylesPlaceholder: bundleStyleContentPlaceholder }), - resolveReact - ], + esbuildPlugins: [...(commonConfig.esbuildPlugins ?? []), resolveReact], format: 'iife', outExtension() { return { js: '.js' }; diff --git a/packages/component/package.json b/packages/component/package.json index 3ce5a4df1e..ad7610253c 100644 --- a/packages/component/package.json +++ b/packages/component/package.json @@ -76,10 +76,12 @@ "homepage": "https://github.com/microsoft/BotFramework-WebChat/tree/main/packages/component#readme", "scripts": { "build": "npm run --if-present build:pre && npm run build:run && npm run --if-present build:post", - "build:post": "npm run build:post:dtsroll && npm run build:post:validate:css && npm run build:post:validate:dts", + "build:post": "npm run build:post:dtsroll && npm run build:post:validate", "build:post:dtsroll": "dtsroll ./dist/*.d.*", + "build:post:validate": "npm run build:post:validate:css && npm run build:post:validate:inject-css && npm run build:post:validate:dts", "build:post:validate:css": "vg ast-check lightning-css ./dist/*.css", "build:post:validate:dts": "vg ast-check dist-types ./dist/*.d.*", + "build:post:validate:inject-css": "vg ast-check css-inject dist/*.js dist/*.mjs", "build:pre": "npm run build:pre:local-dependencies && npm run build:pre:watch", "build:pre:local-dependencies": "../../scripts/npm/build-local-dependencies.sh", "build:pre:watch": "../../scripts/npm/build-watch.sh", diff --git a/packages/component/tsup.config.ts b/packages/component/tsup.config.ts index 9ff33e0e85..23963b16b0 100644 --- a/packages/component/tsup.config.ts +++ b/packages/component/tsup.config.ts @@ -19,8 +19,7 @@ const commonConfig = applyConfig(config => ({ injectCSSPlugin({ // esbuild does not fully support CSS code splitting, every entry point has its own CSS file. // Related to https://github.com/evanw/esbuild/issues/608. - getCSSText: (_source, cssFiles) => - cssFiles.find(({ path }) => path.endsWith('botframework-webchat-component.component.css'))?.text, + ignoreCSSEntries: ['dist/botframework-webchat-component.component.css'], stylesPlaceholder: componentStyleContentPlaceholder }), injectCSSPlugin({ stylesPlaceholder: decoratorStyleContentPlaceholder }) diff --git a/packages/core/package.json b/packages/core/package.json index d9cb31eaba..72ddd1c3e8 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -80,7 +80,7 @@ "build": "npm run --if-present build:pre && npm run build:run && npm run --if-present build:post", "build:post": "npm run build:post:dtsroll && npm run build:post:validate:dts", "build:post:dtsroll": "dtsroll ./dist/*.d.*", - "build:post:validate:dts": "if grep -q -P '@msinternal\\/' dist/*.d.* 2>/dev/null; then echo \"Error: dist/*.d.* is not compiled by dtsroll\" >&2; exit 1; fi", + "build:post:validate:dts": "vg ast-check dist-types ./dist/*.d.*", "build:pre": "npm run build:pre:local-dependencies && npm run build:pre:watch", "build:pre:local-dependencies": "../../scripts/npm/build-local-dependencies.sh", "build:pre:watch": "../../scripts/npm/build-watch.sh", diff --git a/packages/fluent-theme/esbuild.static.mjs b/packages/fluent-theme/esbuild.static.mjs index 2d18829063..305cbb1a23 100644 --- a/packages/fluent-theme/esbuild.static.mjs +++ b/packages/fluent-theme/esbuild.static.mjs @@ -51,6 +51,7 @@ const config = { format: 'esm', loader: { '.js': 'jsx' }, minify: true, + metafile: true, outdir: resolve(fileURLToPath(import.meta.url), `../static/`), platform: 'browser', plugins: [ diff --git a/packages/fluent-theme/package.json b/packages/fluent-theme/package.json index 2482963648..36dee197c4 100644 --- a/packages/fluent-theme/package.json +++ b/packages/fluent-theme/package.json @@ -43,9 +43,10 @@ "build": "npm run --if-present build:pre && npm run build:run && npm run --if-present build:post", "build:post": "npm run build:post:dtsroll && npm run build:post:validate", "build:post:dtsroll": "dtsroll ./dist/*.d.*", - "build:post:validate": "npm run build:post:validate:css && npm run build:post:validate:dts", + "build:post:validate": "npm run build:post:validate:css && npm run build:post:validate:inject-css && npm run build:post:validate:dts", "build:post:validate:css": "vg ast-check lightning-css ./dist/*.css", "build:post:validate:dts": "vg ast-check dist-types ./dist/*.d.*", + "build:post:validate:inject-css": "vg ast-check css-inject dist/*.js dist/*.mjs static/*.js", "build:pre": "npm run build:pre:local-dependencies && npm run build:pre:watch", "build:pre:local-dependencies": "../../scripts/npm/build-local-dependencies.sh", "build:pre:watch": "../../scripts/npm/build-watch.sh", diff --git a/packages/fluent-theme/tsup.config.ts b/packages/fluent-theme/tsup.config.ts index 2786f66cee..4dff24e2e6 100644 --- a/packages/fluent-theme/tsup.config.ts +++ b/packages/fluent-theme/tsup.config.ts @@ -102,7 +102,9 @@ export default defineConfig([ entry: { 'botframework-webchat-fluent-theme.production.min': './src/bundle.ts' }, esbuildPlugins: [ ...(config.esbuildPlugins ?? []), - injectCSSPlugin({ stylesPlaceholder: fluentStyleContentPlaceholder }), + injectCSSPlugin({ + stylesPlaceholder: fluentStyleContentPlaceholder + }), umdResolvePlugin ], format: 'iife', diff --git a/packages/styles/src/build/private/injectCSSPlugin.ts b/packages/styles/src/build/private/injectCSSPlugin.ts index d69e2ad098..9573e1a25c 100644 --- a/packages/styles/src/build/private/injectCSSPlugin.ts +++ b/packages/styles/src/build/private/injectCSSPlugin.ts @@ -1,8 +1,9 @@ import { decode, encode } from '@jridgewell/sourcemap-codec'; +import path from 'node:path'; import type { OutputFile, Plugin } from 'esbuild'; export interface InjectCSSPluginOptions { - getCSSText?: ((source: OutputFile, cssFiles: OutputFile[]) => string | undefined | void) | undefined; + ignoreCSSEntries?: string[]; stylesPlaceholder: string; } @@ -20,63 +21,203 @@ function updateMappings(encoded: string, startIndex: number, offset: number) { return encode(mappings); } -export default function injectCSSPlugin({ getCSSText, stylesPlaceholder }: InjectCSSPluginOptions): Plugin { - if (!stylesPlaceholder) { - throw new Error('inject-css-plugin: no placeholder for styles provided'); +type Metafile = { + outputs: Record< + string, + { + entryPoint?: string; + imports?: Array<{ path: string; kind?: string; external?: boolean }>; + } + >; +}; + +function mapOutputsToRootOutputs(metafile: Metafile) { + const outputs = metafile.outputs || {}; + const allFiles = new Set(Object.keys(outputs)); + + const roots: string[] = []; + const graph = new Map(); + + for (const [file, meta] of Object.entries(outputs)) { + if (meta.entryPoint) { + roots.push(file); + } + + const edges: string[] = []; + const imports = meta.imports || []; + + for (const imp of imports) { + if (imp.external) { + continue; + } + + if (allFiles.has(imp.path)) { + edges.push(imp.path); + } else { + const resolved = path.posix.normalize(path.posix.resolve(path.posix.dirname(file), imp.path)); + if (allFiles.has(resolved)) { + edges.push(resolved); + } + } + } + graph.set(file, edges); } - getCSSText = - getCSSText || - ((source, cssFiles) => { - const entryName = source.path.replace(/(\.js|\.mjs)$/u, ''); - const css = cssFiles.find(f => f.path.replace(/(\.css)$/u, '') === entryName); + roots.sort(); - return css?.text; - }); + const outputToRoots = new Map>(); + + for (const file of allFiles) { + outputToRoots.set(file, new Set()); + } + + for (const root of roots) { + const stack = [root]; + const visited = new Set(); + + while (stack.length > 0) { + const node = stack.pop()!; + + if (visited.has(node)) { + continue; + } + visited.add(node); + + outputToRoots.get(node)?.add(root); + + const children = graph.get(node); + if (children) { + stack.push(...children); + } + } + } + + const result = new Map(); + for (const [file, rootSet] of outputToRoots) { + result.set(file, Object.freeze([...rootSet].sort())); + } + + return { roots, outputToRoots: result }; +} + +function findOutputKeyForFile(filePath: string, outputToRoots: Map): string | undefined { + const fp = path.normalize(filePath); + + // output keys in esbuild metafile are relative e.g. "dist/chunk-XYZ.js" + for (const outKey of outputToRoots.keys()) { + const k1 = path.normalize(outKey); // "dist/chunk-XYZ.js" + const k2 = path.normalize(path.join(path.sep, outKey)); // "/dist/chunk-XYZ.js" + if (fp.endsWith(k1) || fp.endsWith(k2)) { + return outKey; + } + } + + return undefined; +} + +export default function injectCSSPlugin({ ignoreCSSEntries, stylesPlaceholder }: InjectCSSPluginOptions): Plugin { + if (!stylesPlaceholder) { + throw new Error('inject-css-plugin: no placeholder for styles provided'); + } const stylesPlaceholderQuoted = JSON.stringify(stylesPlaceholder); + const ignoreCSSEntriesSet = new Set(ignoreCSSEntries); + return { name: `inject-css-plugin(${stylesPlaceholder})`, setup(build) { - build.onEnd(({ outputFiles = [] }) => { + build.onEnd(({ outputFiles = [], metafile }) => { const cssFiles = outputFiles.filter(({ path }) => path.match(/(\.css)$/u)); + const jsFiles = outputFiles.filter(({ path }) => path.match(/(\.js|\.mjs)$/u)); + + const jsToCssMap = new Map( + cssFiles + .map(cssFile => { + const jsFilePath = jsFiles.find( + jsFile => jsFile.path.replace(/(\.js|\.mjs)$/u, '') === cssFile.path.replace(/(\.css)$/u, '') + )?.path; + if (!jsFilePath) { + return; + } + return [jsFilePath, cssFile] as const; + }) + .filter((entry): entry is readonly [string, OutputFile] => entry !== undefined) + ); + + if (!metafile) { + throw new Error('inject-css-plugin: metafile is required for proper CSS injection'); + } + + const { outputToRoots } = mapOutputsToRootOutputs(metafile); for (const file of outputFiles) { if (file.path.match(/(\.js|\.mjs)$/u)) { - const cssText = getCSSText(file, cssFiles); const jsText = file?.text; - if (cssText && jsText?.includes(stylesPlaceholderQuoted)) { - const index = jsText.indexOf(stylesPlaceholderQuoted); - const map = outputFiles.find(f => f.path.replace(/(\.map)$/u, '') === file.path); + const shouldProccess = jsText?.includes(stylesPlaceholderQuoted); - const updatedJsText = [ - jsText.slice(0, index), - JSON.stringify(cssText), - jsText.slice(index + stylesPlaceholderQuoted.length) - ].join(''); + if (!shouldProccess) { + continue; + } - file.contents = Buffer.from(updatedJsText); + const outKey = findOutputKeyForFile(file.path, outputToRoots); + const owners = (outKey && outputToRoots.get(outKey)) || []; + const cssFilesMap = new Map( + owners + ?.map(owner => { + const cssFile = jsToCssMap.get(path.join(process.cwd(), owner)); + if (!cssFile) { + return; + } + const cssKey = cssFile ? path.relative(process.cwd(), cssFile.path) : undefined; + return [cssKey, cssFile] as const; + }) + .filter((entry): entry is readonly [string, OutputFile] => entry !== undefined) + ); + + const cssCandidateKeys = new Set(cssFilesMap.keys()).difference(ignoreCSSEntriesSet); + + if (cssCandidateKeys.size !== 1) { + throw new Error( + `inject-css-plugin: unable to uniquely determine CSS for ${outKey}. Found CSS entries: \n[\n${[ + ...cssCandidateKeys + ] + .map(entry => ` '${entry}'`) + .join(',\n')}\n]\n Add the appropriate CSS file to ignoreCSSEntries to fix this issue.` + ); + } - // eslint-disable-next-line no-magic-numbers - if (updatedJsText.indexOf(stylesPlaceholder) !== -1) { - throw new Error( - `Duplicate placeholders are not supported.\nFound ${stylesPlaceholder} in ${file.path}.` - ); - } + const cssText = cssFilesMap.get(Array.from(cssCandidateKeys).at(0)!)?.text; - if (map) { - const parsed = JSON.parse(map.text); + if (!cssText) { + throw new Error( + `inject-css-plugin: unable to find CSS text for ${outKey}.${ignoreCSSEntries ? '\n The following entries were ignored:\n' + ignoreCSSEntries.map(entry => ` '${entry}'`).join('\n') : ''}` + ); + } - parsed.mappings = updateMappings( - parsed.mappings, - index, - cssText.length - stylesPlaceholderQuoted.length - ); + const index = jsText.indexOf(stylesPlaceholderQuoted); + const map = outputFiles.find(f => f.path.replace(/(\.map)$/u, '') === file.path); - map.contents = Buffer.from(JSON.stringify(parsed)); - } + const updatedJsText = [ + jsText.slice(0, index), + JSON.stringify(cssText), + jsText.slice(index + stylesPlaceholderQuoted.length) + ].join(''); + + file.contents = Buffer.from(updatedJsText); + + // eslint-disable-next-line no-magic-numbers + if (updatedJsText.indexOf(stylesPlaceholder) !== -1) { + throw new Error(`Duplicate placeholders are not supported.\nFound ${stylesPlaceholder} in ${file.path}.`); + } + + if (map) { + const parsed = JSON.parse(map.text); + + parsed.mappings = updateMappings(parsed.mappings, index, cssText.length - stylesPlaceholderQuoted.length); + + map.contents = Buffer.from(JSON.stringify(parsed)); } } } diff --git a/packages/vibe-grep/src/rules/css-inject.yaml b/packages/vibe-grep/src/rules/css-inject.yaml new file mode 100644 index 0000000000..7a9a877480 --- /dev/null +++ b/packages/vibe-grep/src/rules/css-inject.yaml @@ -0,0 +1,14 @@ +--- + +# This rule checks that CSS is injected properly by verifying there are no placeholders + +id: css-inject-placeholder +language: JavaScript +rule: + kind: string + regex: "\\@\\-\\-[A-Z\\-]+\\-\\-\\@" + +description: "Ensure CSS placeholders are replaced during bundling" +message: "Found CSS placeholder. Ensure CSS is properly injected during bundling." +severity: error +args: [dist/*.js dist/*.mjs static/*.js]