Skip to content

Commit 87f60f8

Browse files
committed
fix(readme-sync): reject noncanonical markers, test the production replacement path
1 parent fb457ea commit 87f60f8

4 files changed

Lines changed: 62 additions & 24 deletions

File tree

‎.github/scripts/sync-package-readmes.mjs‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,19 @@ const countByName = (text, re) => {
5959
* stale block reaches npm.
6060
*/
6161
export function assertWellFormed(text, label, allowed, onError = fail) {
62+
// Canonical form, and the loose form used only to catch near-misses. A typo
63+
// (`links2`, `Links`, `:begin`, stray whitespace) must NOT read as "this
64+
// package opted out" — that would let `--check` pass on stale content.
6265
const MARKER_RE = /<!-- polycss:shared:([a-z-]+):(start|end) -->/g;
66+
const LOOSE_RE = /<!--[^>]*polycss:shared[^>]*-->/g;
67+
68+
for (const m of text.matchAll(LOOSE_RE)) {
69+
if (!new RegExp(`^${MARKER_RE.source}$`).test(m[0]))
70+
return onError(
71+
`${label}: malformed shared marker ${JSON.stringify(m[0])} — expected exactly "<!-- polycss:shared:<name>:start -->" or ":end", with a lowercase-and-hyphen name`,
72+
);
73+
}
74+
6375
const names = new Set();
6476
let open = null;
6577

@@ -93,6 +105,22 @@ export function assertWellFormed(text, label, allowed, onError = fail) {
93105
return names;
94106
}
95107

108+
/**
109+
* Rewrite every shared block present in `text` from `blocks`. This is the
110+
* production replacement path — tests must call THIS, not a local copy.
111+
*/
112+
export function applyBlocks(text, blocks, label, allowed, onError = fail) {
113+
const present = assertWellFormed(text, label, allowed, onError);
114+
if (present === null) return null;
115+
let next = text;
116+
for (const name of present) {
117+
// Replacement passed as a CALLBACK: README content is arbitrary text, and
118+
// `$&` / `` $` `` / `$'` in a replacement string would be expanded.
119+
next = next.replace(blockRe(name), () => blocks.get(name));
120+
}
121+
return next;
122+
}
123+
96124
function main() {
97125
const rootText = readFileSync(source, "utf8");
98126
const names = [...assertWellFormed(rootText, "root README", null)];
@@ -121,14 +149,7 @@ for (const target of targets) {
121149
fail(`${target}: cannot be read (${err.code ?? err.message})`);
122150
}
123151

124-
const present = assertWellFormed(text, target, new Set(names));
125-
126-
let next = text;
127-
for (const name of present) {
128-
// Replacement passed as a callback: README content is arbitrary text and
129-
// `$&` / `` $` `` / `$'` in a replacement STRING would be expanded.
130-
next = next.replace(blockRe(name), () => blocks.get(name));
131-
}
152+
const next = applyBlocks(text, blocks, target, new Set(names));
132153

133154
if (next === text) continue;
134155
if (checkOnly) {

‎.github/scripts/sync-package-readmes.test.mjs‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
import { strict as assert } from "node:assert";
1010
import test from "node:test";
11-
import { assertWellFormed } from "./sync-package-readmes.mjs";
11+
import { applyBlocks, assertWellFormed } from "./sync-package-readmes.mjs";
1212

1313
const S = (n) => `<!-- polycss:shared:${n}:start -->`;
1414
const E = (n) => `<!-- polycss:shared:${n}:end -->`;
@@ -66,16 +66,31 @@ test("rejects a block name the root README does not define", () => {
6666
assert.match(error, /not defined in the root README/);
6767
});
6868

69-
test("replacement copies content literally, including $ sequences", () => {
70-
// Regression: passing the block as a replacement STRING expands `$&`,
71-
// "$`" and `$'`, corrupting any README containing them.
72-
const blockRe = /<!-- polycss:shared:links:start -->[\s\S]*?<!-- polycss:shared:links:end -->/;
73-
const replacement = `${S("links")}\ncost: $5 — see $& and $\` and $'\n${E("links")}`;
69+
test("applyBlocks copies content literally, including $ sequences", () => {
70+
// Regression: a replacement STRING expands `$&`, "$`" and `$'`. This drives
71+
// the PRODUCTION path, so reverting it to the string form fails here.
72+
const block = `${S("links")}\ncost: $5 — see $& and $\` and $'\n${E("links")}`;
7473
const target = `head\n${S("links")}\nold\n${E("links")}\ntail`;
7574

76-
const viaString = target.replace(blockRe, replacement);
77-
const viaCallback = target.replace(blockRe, () => replacement);
75+
const out = applyBlocks(target, new Map([["links", block]]), "fixture", ALLOWED);
7876

79-
assert.ok(viaCallback.includes("$& and $` and $'"), "callback must copy verbatim");
80-
assert.notEqual(viaString, viaCallback, "string form is the buggy path this guards against");
77+
assert.ok(out.includes("$& and $` and $'"), "block must be copied byte-for-byte");
78+
assert.equal(out, `head\n${block}\ntail`);
79+
});
80+
81+
test("applyBlocks leaves a marker-less file untouched", () => {
82+
const target = "# Pkg\n\nnothing shared here\n";
83+
assert.equal(applyBlocks(target, new Map(), "fixture", ALLOWED), target);
84+
});
85+
86+
test("rejects marker-like comments that are not canonical", () => {
87+
for (const bad of [
88+
"<!-- polycss:shared:links2:start -->",
89+
"<!-- polycss:shared:Links:start -->",
90+
"<!-- polycss:shared:links:begin -->",
91+
"<!-- polycss:shared:links:start -->",
92+
]) {
93+
const { error } = check(`head\n${bad}\nx\n${E("links")}\n`);
94+
assert.match(error ?? "", /malformed shared marker/, `should reject ${bad}`);
95+
}
8196
});

‎packages/core/src/types.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,11 @@ export interface PolyAmbientLight {
136136
* Material — paint configuration shareable across many polygons.
137137
*
138138
* In CSS terms, a material bundles the `background-image` source plus paint
139-
* config. When a polygon references a material AND its UVs form an
140-
* axis-aligned rectangle, PolyCSS renders the polygon as an <i> with
141-
* `background-image: url(material.texture)` directly — no per-polygon canvas
142-
* rasterization, browser-cached texture, mounting / unmounting one polygon
143-
* does not affect any other.
139+
* config. Material-backed polygons render through the texture atlas by
140+
* default. A direct image leaf — source URL and rect, no per-polygon canvas
141+
* rasterization — requires valid `imageSource` metadata AND a resolved
142+
* presentation of `backend: "image"` with source lighting; the default
143+
* `"auto"` backend resolves to the atlas.
144144
*
145145
* Three.js parallel: combines THREE.Texture + a basic Material in one. CSS
146146
* has no shader/sampler concerns, so the texture/material split from

‎website/public/skill.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ interface Polygon {
7373
texture?: string; // image URL
7474
uvs?: [number, number][]; // one per vertex
7575
material?: PolyMaterial; // shared material; material.texture wins over `texture`
76-
textureImageSource?: PolyTextureImageSource; // source-exact image leaf (advanced)
76+
textureImageSource?: PolyTextureImageSource; // source image metadata; needs texturePresentation.backend="image" (advanced)
7777
texturePresentation?: PolyTexturePresentation; // per-polygon texture overrides (advanced)
7878
data?: Record<string, string | number | boolean>; // → data-* attributes
7979
}
@@ -179,6 +179,8 @@ createPolyOrbitControls(scene, { drag: true, wheel: true });
179179

180180
scene.add(createPolyBox({ size: 100, color: "#ffd166" }), { position: [0, 0, 50] });
181181
scene.add(await loadMesh("/model.glb"), { castShadow: true });
182+
// Vanilla has no ground fallback — a caster needs an explicit receiver.
183+
scene.add(createPolyPlane({ size: 400, color: "#7d848e" }), { receiveShadow: true });
182184
```
183185

184186
React (Vue mirrors this with kebab-case props):

0 commit comments

Comments
 (0)