Skip to content

Commit f68a007

Browse files
makhnatkinclaude
andcommitted
refactor(shadow-styles): address self-review feedback
Decouple drift-check from gulpfile by extracting SHADOW_STYLE_IMPORTS into scripts/shadow-styles-imports.mjs, so the CI check no longer dynamically imports the gulpfile (which would also run registerBuildTasks side effects) just to read one constant. Broaden the drift-check regex to cover non-side-effect import forms (`import x from 'pkg/x.css'`, `import * as x from`, named imports, and dynamic `import('pkg/x.css')`), and strip block/line comments before matching so commented-out imports do not produce false positives. Split into two regexes (static vs dynamic) for readability. Add an `assertNoCssImportRules` guard in the `shadow-styles` gulp task: `CSSStyleSheet.replaceSync()` rejects `@import` rules at runtime, so fail the build instead of letting Shadow DOM consumers crash. The guard strips comments and string literals first to avoid false positives on `@import` substrings inside CSS values. Fold the standalone `Check Shadow Styles Imports` CI job into the existing `tests` job — saves a redundant pnpm install (~30-60s) per PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 3ecf3c3 commit f68a007

4 files changed

Lines changed: 71 additions & 44 deletions

File tree

.github/workflows/ci.yml

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ jobs:
5656
- name: ESBuild compatability
5757
run: pnpm ci:test:esbuild
5858

59+
- name: Check shadow styles imports
60+
run: pnpm ci:test:shadow-styles
61+
5962
check_circular_deps:
6063
name: Check Circular Dependencies
6164
runs-on: ubuntu-latest
@@ -77,25 +80,3 @@ jobs:
7780

7881
- name: Check circular dependencies
7982
run: pnpm ci:test:circular-deps
80-
81-
check_shadow_styles_imports:
82-
name: Check Shadow Styles Imports
83-
runs-on: ubuntu-latest
84-
steps:
85-
- name: Checkout
86-
uses: actions/checkout@v6
87-
88-
- name: Setup pnpm
89-
uses: pnpm/action-setup@v5
90-
91-
- name: Setup Node
92-
uses: actions/setup-node@v6
93-
with:
94-
node-version-file: '.nvmrc'
95-
cache: pnpm
96-
97-
- name: Install dependencies
98-
run: pnpm run ci:deps
99-
100-
- name: Check shadow styles imports
101-
run: pnpm ci:test:shadow-styles

packages/editor/gulpfile.mjs

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,13 @@ import {parallel, series, task} from '@markdown-editor/gulp-tasks';
77
import {registerBuildTasks} from '@markdown-editor/gulp-tasks/build';
88

99
import pkg from './package.json' with {type: 'json'};
10+
import {SHADOW_STYLE_IMPORTS} from './scripts/shadow-styles-imports.mjs';
1011

1112
const __dirname = dirname(fileURLToPath(import.meta.url));
1213
const require = createRequire(import.meta.url);
1314

1415
const BUILD_DIR = resolve('build');
1516
const NODE_MODULES_DIR = resolve(__dirname, 'node_modules');
16-
// Keep this list aligned with non-relative CSS imports required by the default editor setup.
17-
// Drift is enforced by scripts/check-shadow-styles-imports.js.
18-
export const SHADOW_STYLE_IMPORTS = Object.freeze([
19-
'@diplodoc/transform/dist/css/base.css',
20-
'@diplodoc/transform/dist/css/_yfm-only.css',
21-
'@diplodoc/cut-extension/runtime/styles.css',
22-
'@diplodoc/file-extension/runtime/styles.css',
23-
'@diplodoc/tabs-extension/runtime/styles.css',
24-
'@diplodoc/quote-link-extension/runtime/styles.css',
25-
'@diplodoc/folding-headings-extension/runtime/styles.css',
26-
]);
2717

2818
const OPTIONAL_PEERS = new Set(
2919
Object.entries(pkg.peerDependenciesMeta ?? {})
@@ -41,6 +31,7 @@ task('shadow-styles', (done) => {
4131
const externalCss = readExternalShadowStyles();
4232
const editorCss = readFileSync(resolve(BUILD_DIR, 'styles.css'), 'utf8');
4333
const styles = [externalCss, editorCss].filter(Boolean).join('\n');
34+
assertNoCssImportRules(styles);
4435
const moduleCode = createShadowStylesModule(styles);
4536

4637
writeFileSync(resolve(BUILD_DIR, 'shadow-styles.mjs'), moduleCode.esm);
@@ -77,6 +68,21 @@ function readExternalShadowStyles() {
7768
.join('\n');
7869
}
7970

71+
// `CSSStyleSheet.replaceSync()` rejects `@import` rules. If any inlined CSS ever
72+
// adds one, fail the build instead of letting consumers crash at runtime.
73+
function assertNoCssImportRules(css) {
74+
const stripped = css
75+
.replace(/\/\*[\s\S]*?\*\//g, '')
76+
.replace(/(["'])(?:\\.|(?!\1).)*\1/g, '');
77+
if (/(?:^|[\s;}])@import\b/m.test(stripped)) {
78+
throw new Error(
79+
"[shadow-styles] '@import' rules are not allowed in cssText: " +
80+
'CSSStyleSheet.replaceSync() would reject them at runtime. ' +
81+
'Inline the imported file into SHADOW_STYLE_IMPORTS instead.',
82+
);
83+
}
84+
}
85+
8086
function isOptionalPeerImport(cssImport) {
8187
const pkgName = cssImport.startsWith('@')
8288
? cssImport.split('/', 2).join('/')

packages/editor/scripts/check-shadow-styles-imports.js

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,33 @@ const path = require('node:path');
44
const {pathToFileURL} = require('node:url');
55

66
const SRC_DIR = path.resolve(__dirname, '..', 'src');
7-
const GULPFILE_URL = pathToFileURL(path.resolve(__dirname, '..', 'gulpfile.mjs')).href;
7+
const IMPORTS_MODULE_URL = pathToFileURL(path.resolve(__dirname, 'shadow-styles-imports.mjs')).href;
88

9-
// Bare `import 'pkg/path/file.css';` — non-relative, scoped or unscoped, ending in .css.
10-
const CSS_IMPORT_RE = /(?:^|\s)import\s+['"]((?:@[^'"\s/]+\/)?[^'"\s.][^'"\s]*\.css)['"]/gm;
9+
// Capture group `(...\.css)` — non-relative specifier ending in .css:
10+
// scoped (`@scope/pkg/...`) or unscoped (`pkg/...`). Anchored to start-of-line
11+
// (with optional whitespace) so matches inside string literals can't pollute
12+
// results. Two regexes — static and dynamic — for readability.
13+
const CSS_PATH = "((?:@[^'\"\\s/]+/)?[^'\"\\s.][^'\"\\s]*\\.css)";
14+
15+
// import 'pkg/x.css';
16+
// import x from 'pkg/x.css';
17+
// import * as x from 'pkg/x.css';
18+
// import {x} from 'pkg/x.css';
19+
const STATIC_CSS_IMPORT_RE = new RegExp(
20+
`^\\s*import\\s+(?:[\\w*\\s{},]+\\s+from\\s+)?['"]${CSS_PATH}['"]`,
21+
'gm',
22+
);
23+
24+
// import('pkg/x.css')
25+
// await import('pkg/x.css')
26+
const DYNAMIC_CSS_IMPORT_RE = new RegExp(`\\bimport\\s*\\(\\s*['"]${CSS_PATH}['"]`, 'g');
1127

1228
const EXCLUDED_SCOPES = ['@gravity-ui/'];
1329

1430
async function main() {
15-
const {SHADOW_STYLE_IMPORTS} = await import(GULPFILE_URL);
31+
const {SHADOW_STYLE_IMPORTS} = await import(IMPORTS_MODULE_URL);
1632
if (!Array.isArray(SHADOW_STYLE_IMPORTS)) {
17-
console.error('Failed to import SHADOW_STYLE_IMPORTS from gulpfile.mjs');
33+
console.error('Failed to import SHADOW_STYLE_IMPORTS from shadow-styles-imports.mjs');
1834
process.exit(1);
1935
}
2036

@@ -34,7 +50,7 @@ async function main() {
3450
console.error(' Stale in SHADOW_STYLE_IMPORTS (in list, not used in src):');
3551
for (const item of stale) console.error(` - ${item}`);
3652
}
37-
console.error('Update SHADOW_STYLE_IMPORTS in packages/editor/gulpfile.mjs.');
53+
console.error('Update SHADOW_STYLE_IMPORTS in packages/editor/scripts/shadow-styles-imports.mjs.');
3854
process.exit(1);
3955
}
4056

@@ -46,16 +62,27 @@ function collectCssImports(dir) {
4662
const result = new Set();
4763
walk(dir, (filePath) => {
4864
if (!/\.(?:ts|tsx|js|jsx|mjs|cjs)$/.test(filePath)) return;
49-
const content = fs.readFileSync(filePath, 'utf8');
50-
for (const match of content.matchAll(CSS_IMPORT_RE)) {
51-
const spec = match[1];
52-
if (EXCLUDED_SCOPES.some((scope) => spec.startsWith(scope))) continue;
53-
result.add(spec);
65+
const content = stripComments(fs.readFileSync(filePath, 'utf8'));
66+
for (const re of [STATIC_CSS_IMPORT_RE, DYNAMIC_CSS_IMPORT_RE]) {
67+
for (const match of content.matchAll(re)) {
68+
const spec = match[1];
69+
if (EXCLUDED_SCOPES.some((scope) => spec.startsWith(scope))) continue;
70+
result.add(spec);
71+
}
5472
}
5573
});
5674
return result;
5775
}
5876

77+
// Strip block and line comments so commented-out imports don't show up as
78+
// drift. Naive replace — acceptable because we never *add* matches by stripping,
79+
// only remove potential matches. The only edge case is a regex literal like
80+
// `/foo\/\/bar/` whose `//` would be mistaken for a line comment, but no
81+
// `import 'pkg/x.css'` can live inside a regex literal anyway.
82+
function stripComments(source) {
83+
return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/\/\/[^\n\r]*/g, '');
84+
}
85+
5986
function walk(dir, visit) {
6087
for (const entry of fs.readdirSync(dir, {withFileTypes: true})) {
6188
const full = path.join(dir, entry.name);
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Single source of truth for non-relative CSS imports inlined into shadow-styles.
2+
// Imported by both gulpfile.mjs (build task) and check-shadow-styles-imports.js
3+
// (CI drift check). Keep this list aligned with imports under packages/editor/src.
4+
5+
export const SHADOW_STYLE_IMPORTS = Object.freeze([
6+
'@diplodoc/transform/dist/css/base.css',
7+
'@diplodoc/transform/dist/css/_yfm-only.css',
8+
'@diplodoc/cut-extension/runtime/styles.css',
9+
'@diplodoc/file-extension/runtime/styles.css',
10+
'@diplodoc/tabs-extension/runtime/styles.css',
11+
'@diplodoc/quote-link-extension/runtime/styles.css',
12+
'@diplodoc/folding-headings-extension/runtime/styles.css',
13+
]);

0 commit comments

Comments
 (0)