Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ packages/icons/vjsc/schema.generated.ts
packages/react/vjsc/entries.generated.ts
packages/skins/vjsc/registry/
# Framework Skin sources are generated from the canonical VJSC component graph.
packages/html/src/presets/background/skin.ts
packages/html/src/define/background/skin.css
packages/html/src/internal/skins/
packages/react/src/internal/skins/
packages/react/src/presets/*/skin.tsx
packages/react/src/presets/*/skin.css
Expand Down
3 changes: 1 addition & 2 deletions packages/html/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
"access": "public"
},
"scripts": {
"check:cdn": "node --import tsx ./scripts/check-cdn-self-contained.ts",
"check:cdn": "node --import tsx ./scripts/check-cdn-self-contained.ts && node --import tsx ./scripts/check-cdn-skins.ts",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skin registers omitted from sideEffects

High Severity

Generated skin register modules are side-effect-only (bare define/ui/* imports plus registerIcons()), but package.json sideEffects still lists only define/**, i18n, and icon element paths. The Vite pack config now treats /internal/skins/*/register as side-effectful so this package's own build keeps them; consumer bundlers that honor sideEffects (webpack, Vite/Rollup production) do not. Importing a skin or preset can drop the register graph, leave template tags as plain HTMLElement, and throw on methods such as setSyncedText.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d3b2371. Configure here.

"build:archive": "node --import tsx ./scripts/build-dist-archive.ts",
"build:watch": "vp pack --filter package --watch ./src --no-clean",
"dev": "pnpm run build:watch",
Expand All @@ -207,7 +207,6 @@
"devDependencies": {
"@testing-library/dom": "^10.4.1",
"@videojs/icons": "workspace:*",
"@videojs/skins": "workspace:*",
"happy-dom": "^20.11.1",
"rolldown": "~1.2.5",
"tsx": "^4.23.1",
Expand Down
177 changes: 177 additions & 0 deletions packages/html/scripts/check-cdn-skins.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/** Verify that every generated skin survives the public CDN build as a browser-ready entry. */

import { existsSync, readFileSync } from 'node:fs';
import { dirname, relative, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';

import { resolveClosure } from './cdn-graph.ts';

const PACKAGE_DIR = resolve(dirname(fileURLToPath(import.meta.url)), '..');
const CDN_DIR = resolve(PACKAGE_DIR, 'cdn');
const PREFIX = '\x1b[35m[check-cdn-skins]\x1b[0m';
const forbiddenRuntime = /(?:virtual:vjsc|vjsc\/components|vjsc\/target|@videojs\/core\/vjsc)/;
const skins = [
{
entry: 'video',
generated: 'default-video',
skinTag: 'video-skin',
playerTag: 'video-player',
scope: '.media-skin--video',
},
{
entry: 'video-minimal',
generated: 'minimal-video',
skinTag: 'video-minimal-skin',
playerTag: 'video-player',
scope: '.media-skin--minimal.media-skin--video',
},
{
entry: 'audio',
generated: 'default-audio',
skinTag: 'audio-skin',
playerTag: 'audio-player',
scope: '.media-skin--audio',
},
{
entry: 'audio-minimal',
generated: 'minimal-audio',
skinTag: 'audio-minimal-skin',
playerTag: 'audio-player',
scope: '.media-skin--minimal.media-skin--audio',
},
{
entry: 'live-video',
generated: 'default-live-video',
skinTag: 'live-video-skin',
playerTag: 'live-video-player',
scope: '.media-skin--live-video',
},
{
entry: 'live-video-minimal',
generated: 'minimal-live-video',
skinTag: 'live-video-minimal-skin',
playerTag: 'live-video-player',
scope: '.media-skin--minimal.media-skin--live-video',
},
{
entry: 'live-audio',
generated: 'default-live-audio',
skinTag: 'live-audio-skin',
playerTag: 'live-audio-player',
scope: '.media-skin--live-audio',
},
{
entry: 'live-audio-minimal',
generated: 'minimal-live-audio',
skinTag: 'live-audio-minimal-skin',
playerTag: 'live-audio-player',
scope: '.media-skin--minimal.media-skin--live-audio',
},
] as const;

function main(): void {
if (!existsSync(CDN_DIR)) fail(`CDN build not found at ${CDN_DIR}. Run \`pnpm build:cdn\` first.`);

for (const skin of skins) {
const stylesheet = readCdn(`${skin.entry}.css`);

assert(stylesheet.length > 10_000, `${skin.entry}.css is unexpectedly incomplete`);
assert(stylesheet.includes(skin.scope), `${skin.entry}.css is missing its skin scope`);

const registration = readSource(`src/internal/skins/${skin.generated}/register.ts`);
const expectedSources = registrationImports(registration);

for (const mode of ['dev', 'prod'] as const) {
const entry = `${skin.entry}${mode === 'dev' ? '.dev' : ''}.js`;
const closure = resolveClosure(CDN_DIR, [entry]);
const source = [...closure].map(readCdn).join('\n');

assert(source.includes(skin.skinTag), `${entry} does not register ${skin.skinTag}`);
assert(source.includes(skin.playerTag), `${entry} does not register ${skin.playerTag}`);
assert(source.includes('<media-container'), `${entry} does not contain its static skin template`);
assert(!forbiddenRuntime.test(source), `${entry} retains a VJSC compiler runtime reference`);
assert(existsSync(resolve(CDN_DIR, `${entry}.map`)), `${entry} is missing its source map`);
assert(readCdn(entry).includes(`sourceMappingURL=${entry}.map`), `${entry} does not reference its source map`);

const sources = mappedSources(closure);
const generatedRegister = `src/internal/skins/${skin.generated}/register.ts`;

assert(
sources.some((value) => value.endsWith(generatedRegister)),
`${entry} dropped ${generatedRegister}`
);

for (const expected of expectedSources) {
assert(
sources.some((value) => value.endsWith(expected)),
`${entry} dropped ${expected}`
);
}
}
}

console.log(PREFIX, `✅ ${skins.length} skins have dev/prod bundles, complete CSS, and source maps`);
}

function readCdn(file: string): string {
const path = resolve(CDN_DIR, file);

if (!existsSync(path)) fail(`Expected CDN skin artifact is missing: ${file}`);

return readFileSync(path, 'utf8');
}

function readSource(file: string): string {
const path = resolve(PACKAGE_DIR, file);

if (!existsSync(path)) fail(`Expected generated skin source is missing: ${file}`);

return readFileSync(path, 'utf8');
}

function mappedSources(files: Iterable<string>): string[] {
const sources = new Set<string>();

for (const file of files) {
const map = resolve(CDN_DIR, `${file}.map`);
if (!existsSync(map)) continue;

const parsed: unknown = JSON.parse(readFileSync(map, 'utf8'));
if (Object.prototype.toString.call(parsed) !== '[object Object]') continue;

// SAFETY: The parsed source map was checked to be an object before reading its optional sources field.
const sourceMap = parsed as { sources?: unknown };
if (!Array.isArray(sourceMap.sources)) continue;

for (const source of sourceMap.sources) {
if (Object.prototype.toString.call(source) !== '[object String]') continue;

sources.add(String(source));
}
}

return [...sources];
}

function registrationImports(registration: string): string[] {
return [...registration.matchAll(/^import ['"]([^'"]+)['"];$/gm)].map((match) => {
const source = resolve(PACKAGE_DIR, 'src/internal/skins/default-video', match[1]!);

return (
relative(PACKAGE_DIR, source)
.replaceAll('\\', '/')
.replace(/\.[cm]?[jt]s$/, '') + '.ts'
);
});
}

function assert(condition: boolean, message: string): asserts condition {
if (!condition) fail(message);
}

function fail(message: string): never {
console.error(PREFIX, '\x1b[31merror:\x1b[0m', message);
process.exit(1);
}

main();
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/minimal-skin.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import "../global.css";
@import "../shared.css";
@import "@videojs/skins/minimal/css/audio.css";
@import "../../internal/skins/minimal-audio/skin.css";
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/minimal-skin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MinimalAudioSkinElement } from '../../presets/audio/minimal-skin';
import { safeDefine } from '../../registration/safe-define';
import './minimal-ui';
import '../../internal/skins/minimal-audio/register';

safeDefine(MinimalAudioSkinElement);
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/skin.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import "../global.css";
@import "../shared.css";
@import "@videojs/skins/default/css/audio.css";
@import "../../internal/skins/default-audio/skin.css";
2 changes: 1 addition & 1 deletion packages/html/src/define/audio/skin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AudioSkinElement } from '../../presets/audio/skin';
import { safeDefine } from '../../registration/safe-define';
import './ui';
import '../../internal/skins/default-audio/register';

safeDefine(AudioSkinElement);
2 changes: 1 addition & 1 deletion packages/html/src/define/live-audio/minimal-skin.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import "../global.css";
@import "../shared.css";
@import "@videojs/skins/minimal/css/audio.css";
@import "../../internal/skins/minimal-live-audio/skin.css";
2 changes: 1 addition & 1 deletion packages/html/src/define/live-audio/minimal-skin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MinimalLiveAudioSkinElement } from '../../presets/live-audio/minimal-skin';
import { safeDefine } from '../../registration/safe-define';
import './minimal-ui';
import '../../internal/skins/minimal-live-audio/register';

safeDefine(MinimalLiveAudioSkinElement);
2 changes: 1 addition & 1 deletion packages/html/src/define/live-audio/skin.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import "../global.css";
@import "../shared.css";
@import "@videojs/skins/default/css/audio.css";
@import "../../internal/skins/default-live-audio/skin.css";
2 changes: 1 addition & 1 deletion packages/html/src/define/live-audio/skin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LiveAudioSkinElement } from '../../presets/live-audio/skin';
import { safeDefine } from '../../registration/safe-define';
import './ui';
import '../../internal/skins/default-live-audio/register';

safeDefine(LiveAudioSkinElement);
2 changes: 1 addition & 1 deletion packages/html/src/define/live-video/minimal-skin.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import "../global.css";
@import "../shared.css";
@import "@videojs/skins/minimal/css/video.css";
@import "../../internal/skins/minimal-live-video/skin.css";
2 changes: 1 addition & 1 deletion packages/html/src/define/live-video/minimal-skin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MinimalLiveVideoSkinElement } from '../../presets/live-video/minimal-skin';
import { safeDefine } from '../../registration/safe-define';
import './minimal-ui';
import '../../internal/skins/minimal-live-video/register';

safeDefine(MinimalLiveVideoSkinElement);
2 changes: 1 addition & 1 deletion packages/html/src/define/live-video/skin.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@import "../global.css";
@import "../shared.css";
@import "@videojs/skins/default/css/video.css";
@import "../../internal/skins/default-live-video/skin.css";
2 changes: 1 addition & 1 deletion packages/html/src/define/live-video/skin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LiveVideoSkinElement } from '../../presets/live-video/skin';
import { safeDefine } from '../../registration/safe-define';
import './ui';
import '../../internal/skins/default-live-video/register';

safeDefine(LiveVideoSkinElement);
14 changes: 10 additions & 4 deletions packages/html/src/define/tests/preset-registration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe('preset registration boundaries', () => {
let define: MockInstance;

function registeredSince(offset: number): string[] {
return define.mock.calls.slice(offset).map((call) => call[0] as string);
return define.mock.calls.slice(offset).map(([tag]) => String(tag));
}

beforeAll(() => {
Expand Down Expand Up @@ -32,7 +32,10 @@ describe('preset registration boundaries', () => {

await import('../video/skin');

expect(registeredSince(before)).toEqual(['video-skin']);
const registered = registeredSince(before);

expect(registered).toContain('video-skin');
expect(registered).not.toContain('video-player');
});

it('video/player registers only the player', async () => {
Expand Down Expand Up @@ -63,12 +66,15 @@ describe('preset registration boundaries', () => {
['audio/minimal-skin', 'audio-minimal-skin', () => import('../audio/minimal-skin')],
['live-video/minimal-skin', 'live-video-minimal-skin', () => import('../live-video/minimal-skin')],
['live-audio/minimal-skin', 'live-audio-minimal-skin', () => import('../live-audio/minimal-skin')],
])('%s registers only its skin element', async (_, skinTag, load) => {
])('%s registers its exact UI closure and skin without the player', async (entry, skinTag, load) => {
const before = define.mock.calls.length;

await load();

expect(registeredSince(before)).toEqual([skinTag]);
const registered = registeredSince(before);

expect(registered).toContain(skinTag);
expect(registered).not.toContain(`${entry.split('/')[0]}-player`);
});

it.each([
Expand Down
Loading
Loading