feat: export CSS as string module for Shadow DOM injection#1027
Open
makhnatkin wants to merge 1 commit intomainfrom
Open
feat: export CSS as string module for Shadow DOM injection#1027makhnatkin wants to merge 1 commit intomainfrom
makhnatkin wants to merge 1 commit intomainfrom
Conversation
|
|
|
|
makhnatkin
force-pushed
the
claude/nice-lederberg
branch
from
8ca0e13 to
cee414e
Compare
Reviewer's GuideAdds a new build artifact that exports the editor’s full CSS (internal SCSS + external YFM-related CSS) as a string module ( File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
EXTERNAL_CSS_IMPORT_REapproach hard-codes a particular import shape (single-line, static, bare module,.csssuffix) and may miss or mis-detect imports as code evolves; consider either broadening the regex to handle multiline/named-only imports or switching to a simple AST-based scan so TS/JS formatting changes don’t silently break CSS collection. - In
collectExternalCss,require.resolve(cssImport)will throw if a referenced CSS module is missing; if this is expected to be robust in different consumer setups, you might want to add a clearer error message or guard around unresolved imports so build failures are easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `EXTERNAL_CSS_IMPORT_RE` approach hard-codes a particular import shape (single-line, static, bare module, `.css` suffix) and may miss or mis-detect imports as code evolves; consider either broadening the regex to handle multiline/named-only imports or switching to a simple AST-based scan so TS/JS formatting changes don’t silently break CSS collection.
- In `collectExternalCss`, `require.resolve(cssImport)` will throw if a referenced CSS module is missing; if this is expected to be robust in different consumer setups, you might want to add a clearer error message or guard around unresolved imports so build failures are easier to diagnose.
## Individual Comments
### Comment 1
<location path="packages/editor/gulpfile.mjs" line_range="26" />
<code_context>
nodeModulesDir: NODE_MODULES_DIR,
});
+task('styles-string', (done) => {
+ const externalCss = collectExternalCss();
+ const editorCss = readFileSync(resolve(BUILD_DIR, 'styles.css'), 'utf8');
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the CSS aggregation and helper logic into a separate module so the gulpfile remains a small, declarative task wiring file.
You can keep the new functionality but move the “mini build system” out of the gulpfile so the gulpfile stays small and easy to scan.
### 1. Extract helpers into a separate module
Create a dedicated helper file (e.g. `build/styles-string.mjs`) and move the regex, FS traversal, and template-escaping there:
```js
// build/styles-string.mjs
import {readFileSync, readdirSync} from 'node:fs';
import {createRequire} from 'node:module';
import {extname, resolve} from 'node:path';
const require = createRequire(import.meta.url);
const SOURCE_EXTENSIONS = new Set(['.js', '.jsx', '.ts', '.tsx']);
const EXTERNAL_CSS_IMPORT_RE =
/^\s*import\s+(?:.+?\s+from\s+)?['"]([^./'"][^'"]*\.css)['"];?/gm;
export function createStylesString({buildDir, sourceDir}) {
const externalCss = collectExternalCss(sourceDir);
const editorCss = readFileSync(resolve(buildDir, 'styles.css'), 'utf8');
const styles = [externalCss, editorCss].filter(Boolean).join('\n');
return toTemplateLiteral(styles);
}
function collectExternalCss(sourceDir) {
const cssImports = new Set();
for (const sourceFile of getSourceFiles(sourceDir)) {
const sourceCode = readFileSync(sourceFile, 'utf8');
for (const match of sourceCode.matchAll(EXTERNAL_CSS_IMPORT_RE)) {
cssImports.add(match[1]);
}
}
return Array.from(cssImports)
.map((cssImport) => readFileSync(require.resolve(cssImport), 'utf8'))
.join('\n');
}
function getSourceFiles(dir) {
return readdirSync(dir, {withFileTypes: true})
// drop sort() if deterministic order is not required
.flatMap((entry) => {
const entryPath = resolve(dir, entry.name);
if (entry.isDirectory()) {
return getSourceFiles(entryPath);
}
return SOURCE_EXTENSIONS.has(extname(entry.name)) ? [entryPath] : [];
});
}
function toTemplateLiteral(value) {
return `\`${value
.replace(/\\/g, '\\\\')
.replace(/`/g, '\\`')
.replace(/\$\{/g, '\\${')}\``;
}
```
### 2. Keep the gulpfile focused on task wiring
Then the gulpfile only wires tasks together and delegates the logic:
```js
// gulpfile.mjs
import {writeFileSync} from 'node:fs';
import {dirname, resolve} from 'node:path';
import {fileURLToPath} from 'node:url';
import {parallel, series, task} from '@markdown-editor/gulp-tasks';
import {registerBuildTasks} from '@markdown-editor/gulp-tasks/build';
import {createStylesString} from './build/styles-string.mjs';
import pkg from './package.json' with {type: 'json'};
const __dirname = dirname(fileURLToPath(import.meta.url));
const BUILD_DIR = resolve('build');
const SOURCE_DIR = resolve('src');
registerBuildTasks({
version: pkg.version,
buildDir: BUILD_DIR,
nodeModulesDir: resolve(__dirname, 'node_modules'),
});
task('styles-string', (done) => {
const content = createStylesString({buildDir: BUILD_DIR, sourceDir: SOURCE_DIR});
writeFileSync(resolve(BUILD_DIR, 'styles-string.mjs'), `export default ${content};\n`);
writeFileSync(resolve(BUILD_DIR, 'styles-string.cjs'), `module.exports = ${content};\n`);
writeFileSync(
resolve(BUILD_DIR, 'styles-string.d.ts'),
'declare const styles: string;\nexport default styles;\n',
);
done();
});
task('build', series(parallel('ts', 'json', 'scss'), 'styles-string'));
task('default', series('clean', 'build'));
```
This keeps all behavior intact but:
- The gulpfile returns to being a small, declarative task definition.
- The complex parts (regex parsing, traversal, escaping) live in an isolated module that you can unit test independently.
- Future readers only need to dive into `build/styles-string.mjs` when they care about the details of CSS aggregation.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| nodeModulesDir: NODE_MODULES_DIR, | ||
| }); | ||
|
|
||
| task('styles-string', (done) => { |
There was a problem hiding this comment.
issue (complexity): Consider extracting the CSS aggregation and helper logic into a separate module so the gulpfile remains a small, declarative task wiring file.
You can keep the new functionality but move the “mini build system” out of the gulpfile so the gulpfile stays small and easy to scan.
1. Extract helpers into a separate module
Create a dedicated helper file (e.g. build/styles-string.mjs) and move the regex, FS traversal, and template-escaping there:
// build/styles-string.mjs
import {readFileSync, readdirSync} from 'node:fs';
import {createRequire} from 'node:module';
import {extname, resolve} from 'node:path';
const require = createRequire(import.meta.url);
const SOURCE_EXTENSIONS = new Set(['.js', '.jsx', '.ts', '.tsx']);
const EXTERNAL_CSS_IMPORT_RE =
/^\s*import\s+(?:.+?\s+from\s+)?['"]([^./'"][^'"]*\.css)['"];?/gm;
export function createStylesString({buildDir, sourceDir}) {
const externalCss = collectExternalCss(sourceDir);
const editorCss = readFileSync(resolve(buildDir, 'styles.css'), 'utf8');
const styles = [externalCss, editorCss].filter(Boolean).join('\n');
return toTemplateLiteral(styles);
}
function collectExternalCss(sourceDir) {
const cssImports = new Set();
for (const sourceFile of getSourceFiles(sourceDir)) {
const sourceCode = readFileSync(sourceFile, 'utf8');
for (const match of sourceCode.matchAll(EXTERNAL_CSS_IMPORT_RE)) {
cssImports.add(match[1]);
}
}
return Array.from(cssImports)
.map((cssImport) => readFileSync(require.resolve(cssImport), 'utf8'))
.join('\n');
}
function getSourceFiles(dir) {
return readdirSync(dir, {withFileTypes: true})
// drop sort() if deterministic order is not required
.flatMap((entry) => {
const entryPath = resolve(dir, entry.name);
if (entry.isDirectory()) {
return getSourceFiles(entryPath);
}
return SOURCE_EXTENSIONS.has(extname(entry.name)) ? [entryPath] : [];
});
}
function toTemplateLiteral(value) {
return `\`${value
.replace(/\\/g, '\\\\')
.replace(/`/g, '\\`')
.replace(/\$\{/g, '\\${')}\``;
}2. Keep the gulpfile focused on task wiring
Then the gulpfile only wires tasks together and delegates the logic:
// gulpfile.mjs
import {writeFileSync} from 'node:fs';
import {dirname, resolve} from 'node:path';
import {fileURLToPath} from 'node:url';
import {parallel, series, task} from '@markdown-editor/gulp-tasks';
import {registerBuildTasks} from '@markdown-editor/gulp-tasks/build';
import {createStylesString} from './build/styles-string.mjs';
import pkg from './package.json' with {type: 'json'};
const __dirname = dirname(fileURLToPath(import.meta.url));
const BUILD_DIR = resolve('build');
const SOURCE_DIR = resolve('src');
registerBuildTasks({
version: pkg.version,
buildDir: BUILD_DIR,
nodeModulesDir: resolve(__dirname, 'node_modules'),
});
task('styles-string', (done) => {
const content = createStylesString({buildDir: BUILD_DIR, sourceDir: SOURCE_DIR});
writeFileSync(resolve(BUILD_DIR, 'styles-string.mjs'), `export default ${content};\n`);
writeFileSync(resolve(BUILD_DIR, 'styles-string.cjs'), `module.exports = ${content};\n`);
writeFileSync(
resolve(BUILD_DIR, 'styles-string.d.ts'),
'declare const styles: string;\nexport default styles;\n',
);
done();
});
task('build', series(parallel('ts', 'json', 'scss'), 'styles-string'));
task('default', series('clean', 'build'));This keeps all behavior intact but:
- The gulpfile returns to being a small, declarative task definition.
- The complex parts (regex parsing, traversal, escaping) live in an isolated module that you can unit test independently.
- Future readers only need to dive into
build/styles-string.mjswhen they care about the details of CSS aggregation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds `@gravity-ui/markdown-editor/styles-string` — a JS module that exports all editor styles as a string, enabling injection into Shadow DOM via `adoptedStyleSheets` or a `<style>` tag.
```ts
import editorStyles from '@gravity-ui/markdown-editor/styles-string';
// Option 1: Constructable Stylesheets (modern browsers)
const sheet = new CSSStyleSheet();
sheet.replaceSync(editorStyles);
shadowRoot.adoptedStyleSheets = [sheet];
// Option 2: <style> tag (broader compatibility)
const style = document.createElement('style');
style.textContent = editorStyles;
shadowRoot.appendChild(style);
```
What's included
The exported string contains:
Note: `@gravity-ui/uikit` styles are not included — if your shadow root also uses UIKit components, you'll need to inject those separately.
Implementation details
Test plan
Closes #1026
Hey @zeeeeby — is this what you had in mind? The exported string covers core editor styles plus the default YFM preset's external dependencies. Let me know if anything is missing from your specific setup (e.g. which extensions/preset you're using, whether you also need UIKit styles injected).
🤖 Generated with Claude Code
Summary by Sourcery
Export compiled editor CSS (including external style dependencies) as a string module and integrate it into the editor build and test pipeline.
New Features:
Enhancements:
Build:
Tests: