Skip to content

chore: v0.42.0-alpha.9 cleanup train (#632-#644) #645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SisyphusZheng merged 10 commits into from
Jul 30, 2026
Merged

chore: v0.42.0-alpha.9 cleanup train (#632-#644) #645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bf7f0df
fix(element): alpha.9 quality cleanup (#633 #639 #640 #643 #644)
Jul 30, 2026
b5c3747
refactor(app): render-scoped data context + logger + spa tagName (#63…
Jul 30, 2026
ed79953
refactor(ui): split OpenButton handlers + drop default exports (#637 …
Jul 30, 2026
b6e9c4a
fix(adapter-vite): support named export for package islands (#638 reg…
Jul 30, 2026
4e01ff3
chore(create): align deno.json tasks + add test task (#635 #641)
Jul 30, 2026
6b71666
test(www): read named OpenButton export in hydration e2e (#638)
Jul 30, 2026
98d9ae8
docs: add alpha.9 audit + executable plan (#632-#644)
Jul 30, 2026
86393d7
fix(www): resolve @openelement/element in e2e bare bundle (#636 fallout)
Jul 30, 2026
5501295
fix(ci): green autoflow:ci sub-gates for alpha.9
Jul 30, 2026
e7ff61f
fix(release): full alpha.9 version bump so CI gates pass
Jul 30, 2026
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Elements are the durable application contract; JSX and Basic Element are the
authoring layer; Declarative Shadow DOM is the default server representation;
interactive regions upgrade selectively.

Published package line: `0.42.0-alpha.8` (`v0.42.0-alpha.8`) — the stable five-package
Published package line: `0.42.0-alpha.9` (`v0.42.0-alpha.9`) — the stable five-package
release under ADR-0119's scoped interface freeze; the abandoned beta naming
is not an active line.

Expand Down
2 changes: 1 addition & 1 deletion README.zh.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
Elements 是可长期保存的应用组件模型；JSX 与 Basic Element 是作者层；
Declarative Shadow DOM 是默认服务端表示；交互区域按需升级。

已发布包线为 `0.42.0-alpha.8`（`v0.42.0-alpha.8`）——ADR-0119 范围化接口冻结下的
已发布包线为 `0.42.0-alpha.9`（`v0.42.0-alpha.9`）——ADR-0119 范围化接口冻结下的
stable 五包版本；已放弃的 beta 命名不再是当前版本线。

## 当前产品
Expand Down
68 changes: 68 additions & 0 deletions docs/audit/2026-07-30-packages-code-review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# 2026-07-30 packages/ Code Review (verified)

> Scope: `packages/element`, `packages/app`, `packages/adapter-vite`,
> `packages/create`, `packages/ui` — source line `v0.42.0-alpha.8`, `main`.\
> Method: external review report, then every finding re-verified against
> source (files, line numbers, and claims read directly).\
> Disposition: all 11 findings filed as issues #632–#642, milestone
> `v0.42.0-alpha.9`. Plan: `docs/release/v0.42.0-alpha.9-plan.md`.

## Overall verdict

**Quality grade: B+ (good).** Clear layering across the five packages:
`element` = zero-dep runtime core, `app` = authoring API, `adapter-vite` =
build orchestration, `create` = scaffolding, `ui` = component library.
Verified: no `as any` and no TODO/FIXME anywhere in `packages/*/src`;
strong security posture (prototype-pollution guards, CSP nonce validation,
HTML escaping); ADR-referenced comments throughout.

## Findings (verified)

| ID | Sev (final) | Finding | Location | Issue |
| --- | ---------------------------------- | --------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------- | -------------------------------------------------------------- |
| H-1 | **Medium** (downgraded, see below) | Module-level `dataStack` render context | `app/src/internal/router/data-context-store.ts` | [#632](https://github.com/open-element/openelement/issues/632) |
| H-2 | High | `escapeHtml` (map single-pass) vs `escapeAttr` (5 chained replaces) dual implementation | `element/src/internal/core/html-escape.ts:32-45` | [#633](https://github.com/open-element/openelement/issues/633) |
| M-1 | Medium | 1-line re-export layer `internal/router/internal/data-context.ts` | `app/src/internal/router/internal/data-context.ts` | [#634](https://github.com/open-element/openelement/issues/634) |
| M-2 | Medium | Per-package task matrix inconsistent (only `element` complete) | `packages/*/deno.json` | [#635](https://github.com/open-element/openelement/issues/635) |
| M-3 | Medium | Raw `console.error` in client-router/spa instead of `createLogger` | `app/src/internal/router/client-router.ts:377,380,487`; `app/src/spa.ts:64,204,211` | [#636](https://github.com/open-element/openelement/issues/636) |
| M-4 | Medium | `OpenButton._handleClick` ~50 lines, multiple responsibilities | `ui/src/open-button.tsx:232-281` | [#637](https://github.com/open-element/openelement/issues/637) |
| M-5 | Medium | Unused `export default` on all 10 UI components | `ui/src/*.tsx` | [#638](https://github.com/open-element/openelement/issues/638) |
| L-1 | Low | `_getLocale` reads `locale` via `as Record<string, unknown>` cast | `element/src/open-element-implementation.ts:599` | [#639](https://github.com/open-element/openelement/issues/639) |
| L-2 | Low | Redundant Fragment string-compare branch — at **3 sites**, not 1 | `render-ir.ts:216`, `jsx-render-dom.ts:328`, `event-hydration.ts:86` | [#640](https://github.com/open-element/openelement/issues/640) |
| L-3 | Low | `create` has `__tests__/cli.test.ts` but no `test` task | `packages/create/deno.json` | [#641](https://github.com/open-element/openelement/issues/641) |
| L-4 | Low | Unknown `tagName` in SPA `renderComponent` fails silently (blank page) | `app/src/spa.ts:70-92` | [#642](https://github.com/open-element/openelement/issues/642) |

## Verification corrections to the original report

1. **H-1 downgraded High → Medium.** The claimed "concurrent SSR stack-frame
corruption" path does not exist: `authoring.ts:296-321` wraps
push → `definition.render()` → pop in a **synchronous try/finally window**
(`render()` is not async), so single-threaded JS cannot interleave two
requests inside it. Residual (real) risk is narrower: hooks called by
function components evaluated later in the async `renderToNode` phase may
read an empty/foreign frame. `AsyncLocalStorage` remains the right
long-term direction, as an architecture evolution item.
2. **M-2 risk wording softened.** Root `deno.json` `test` is a bare recursive
`deno test`, so `app`/`ui`/`create` tests DO run via the root task. The
gap is per-package DX and explicit CI invocation — not silent test loss.
3. **L-2 undercounted.** The redundant Fragment detection appears at 3 sites,
not 1. `Fragment = Symbol.for('openelement.fragment')` uses the global
symbol registry, so the string-compare branch is redundant absent a
cross-realm scenario.

## Cross-cutting patterns

| Pattern | Sites | Governance |
| --------------------------- | ---------------------------------------------------- | ------------ |
| Non-unified logging | app (console) vs element/adapter-vite (createLogger) | #636 |
| Dual escape implementations | escapeHtml / escapeAttr | #633 |
| UI default exports | 10 component files | #638 |
| Package task drift | 5 packages, 5 shapes | #635 (+#641) |

## Dimensions not judged (missing evidence)

- Actual per-package CI gate coverage (workflows drive tests via autoflow
tooling, not direct `deno task test` calls).
- npm publish consumer resolution of all subpath exports.
- Whether adapter-vite SSG rendering serializes requests (relevant to the
residual H-1 risk only).
87 changes: 87 additions & 0 deletions docs/audit/2026-07-30-team-review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# OpenElement packages/ 团队交叉审查报告（2026-07-30）

> 本文档是 **团队版独立审查记录**，与 `2026-07-30-packages-code-review.md`（单轮核实版）互为补充。
> 团队：主理人 齐活林（Delivery Director）｜架构师 高见远｜工程师 寇豆码｜QA 严过关。
> 审查性质：**仅审查，不修改任何文件**。三方各自独立读源码，结论经主理人汇总对照。

## 方法论与前提说明

- 上一轮（2026-07-30 单审）由主理人一人逐条核实，结论已写入 `2026-07-30-packages-code-review.md` 并发出 11 个 issue（milestone `v0.42.0-alpha.9`）。
- 本轮由三位成员**独立**重读源码交叉验证，不以主理人结论为前提（仅作为对照锚点）。
- 环境限制：本运行时未注册 `software-architect` / `software-engineer` / `software-qa-engineer` 自定义 agent 类型，spawn 阶段即失败。已用基础 `general-purpose` agent 等价替代，角色职责在 prompt 内显式指定，团队归属不变。结论有效性不受影响。

## 交叉审查结论总表

| 条目 | 单审结论 | 团队结论（三方） | 偏差 / 校正 |
| --------------------------- | ------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------ |
| **H-1** dataStack 并发 SSR | 主路径不成立，High→Medium；残余=async 阶段读空帧 | 架构师：主路径**不成立**（同步 try/finally 内无 await 间隙）；残余风险**重新定性为单请求作用域 bug**（嵌套非页面组件序列化期调用 `useLoaderData` 读空栈），**非并发竞态** | ✅ 方向一致，归因更精确：是作用域缺陷，不是并发 |
| **H-2** escape 双实现 | 属实，成立 | 工程师：属实但**非安全问题**（结果正确，仅实现不一致）→ **Low** | ⚠️ **严重度应降**：原报告/issue #633 标 High，团队独立判定 Low（维护性） |
| **M-1** 冗余间接层 | 属实 | 工程师：属实 Low | 一致 |
| **M-2** task 不一致 | 表格属实；"测试不会执行"表述过重 | QA：表格属实；根 `deno test` 无路径→递归发现，CI 门 `test:coverage:check` 同样递归，**不漏跑**；真正盲区在 M-4/L-4 边界 | ✅ 一致；措辞应改为"仅影响单包本地 `deno task test`" |
| **M-3** console.error | 属实 | 工程师：属实 Medium；补充"跨包不一致"（app 无自身 `createLogger`） | 一致 |
| **M-4** _handleClick 职责重 | 属实 | QA：属实中，提交路径难孤立测试；工程师：属实 Medium + 4 个额外异味 | 一致，且发现增量问题（见下） |
| **M-5** default export | 属实（原报 10 个组件） | 工程师：属实 Low，**实际 11 处** `export default`（`index.ts` 仅 named 导出，无消费者） | 数量校正：11 处 |
| **L-1** locale cast | 属实 | 工程师：属实（弱）Low | 一致 |
| **L-2** Fragment 冗余 | 补为 3 处 | 工程师：确认 **3 处**（`render-ir.ts:216` / `jsx-render-dom.ts:328` / `event-hydration.ts:86`） | 一致 |
| **L-3** create 缺 test task | 属实 | QA：属实 Low（`cli.test.ts` 179 行存在，deno.json 无 test task） | 一致 |
| **L-4** spa.ts tagName | 属实（"静默失败"） | 工程师：**描述需修正**——非法字符 `tagName` 会**抛 `SyntaxError`**（非静默）；未注册自定义元素才静默空渲染；QA：边界未被覆盖是盲区 | ⚠️ 描述精确化：区分"非法字符抛错"与"未注册静默" |

## 三方独立贡献 / 增量发现

### 架构师（高见远）—— 并发与进程级单例

- **F1** `dataStack` 跨请求交错覆盖：**主路径不成立**。证据：`authoring.ts:298` push → `definition.render` → `popData:319` 全在同步 `try/finally` 内；`callComponent` 同步执行，`await` 在求值之后，单线程无间隙可插入。
- **F2** 残余风险重定性：嵌套"非页面"组件在序列化期调用 `useLoaderData` → 读空栈，属**单请求作用域缺陷**，非并发竞态。
- **F3** `logger.ts` `_warned` 进程级 `Set`：`warnOnce` 一旦触发即**永久抑制**该进程内其他请求/SSG 多页同名告警 → Low。
- **F4** `errors.ts` `_telemetryHook` 全局单例：若每请求 `setErrorTelemetryHook` 则 **last-writer-wins 竞态 + 跨租户泄漏** → Low（启动一次性）/ High（每请求 set）。建议改为请求级上下文。
- 架构健康度：**B+**——依赖方向 `adapter-vite → element/app`、`app → element`、`ui → element` 单向无环，`element` 零依赖纯净。

### 工程师（寇豆码）—— 实现质量与新增异味

- H-2 / M-1 / M-3 / M-4 / M-5 / L-1 / L-2 / L-4 逐条核实见总表。
- **M-4 关联 4 个额外代码异味**（均 `packages/ui/src/open-button.tsx` / `packages/app/src/spa.ts`）：
1. `spa.ts:78` `if (!route.tagName) return;` **死代码守卫不可达**——`RouteConfig.tagName` 必填（`client-router.ts:17`），`:72` 强转 `{tagName?: string}` 掩盖事实。
2. `open-button.tsx:178` `onClick={(e)=>this._handleClick(e)}` **多余箭头包装**；同文件 `:165` 锚点分支直接传引用，风格不一致。
3. `_handleClick` 在 `href`（锚点 `<a>`）分支仍可能触发表单提交（若 `type="submit"` 则 `form.dispatchEvent`），**`<a>` 上语义不当**。
4. `escapeHtml` 对 non-string 返回 `''`（`:33`），而 `escapeAttrValue` 经 `String()` 转换（`:48-50`）——**同包两种空值约定**。

### QA（严过关）—— 测试覆盖与边界

- 覆盖率门禁**真实存在**：阈值 lines 73 / branch 82 / func 77（`check-coverage.ts:73`、`deno.json:97`），scope = `packages/*/src` + `tools/lib`。
- 根 `deno test` 与 CI 门均**无路径参数→递归发现**，`app/ui/create` 的 `__tests__/` 全部被跑入 → **M-2「测试不会被执行」不成立**。
- 真正漏测盲区：**M-4 `_handleClick` 提交路径难孤立测试**、**L-4 `spa.ts` 无效 tagName / 空白页 / `applyPageHostData` 静默失败边界无覆盖**。建议保留门禁并补这两条路径单测，而非仅为包补 test task。

## 整体评级

**维持 B+**。三方独立结论一致支持单审的"报告代码事实基本属实、总体结论可信"判断，但严重度整体应**下调**：

- H-1 由 High 降 Medium（且残余风险重新定性为作用域 bug）；
- H-2 由 High 降 **Low**（维护性问题，非安全缺口）；
- 其余 Medium/Low 与单审一致。

## 对 2026-07-29/30 已发 issue 的校正建议

| Issue | 当前 | 建议校正 |
| -------- | -------------------------- | ---------------------------------------------------------------------------------------------------------------------- |
| #632 H-1 | Medium（已含同步窗口证据） | 补一句：残余风险是**单请求作用域缺陷**而非并发竞态（采纳 F2） |
| #633 H-2 | **High / Gate 1 必须** | ⚠️ 降为 **Low**（团队独立判定非安全） |
| #634 M-1 | Low | 数量无关，保持 |
| #635 M-2 | Medium | 描述改为"单包本地 task 缺失，CI 不漏跑" |
| #636 M-3 | Medium | 补"跨包不一致" |
| #637 M-4 | Medium | 补 4 个关联异味（死代码守卫 / 多余箭头 / `<a>` submit 语义 / 空值约定） |
| #638 M-5 | Low | 数量更正：11 处 `export default` |
| #639 L-1 | Low | 保持 |
| #640 L-2 | Low | 确认 3 处（补 `jsx-render-dom.ts:328`、`event-hydration.ts:86`） |
| #641 L-3 | Low | 保持 |
| #642 L-4 | Low | 描述精确化：非法字符抛 `SyntaxError` / 未注册静默空渲染 |
| — | — | **建议新增** F3 `_warned` 进程级单例（Low）、F4 `_telemetryHook` 全局单例竞态（Low/High）—— 可并入 #632 或单独开 issue |

## 整改优先级重排（团队版）

1. **必须（安全/正确性）**：F2 嵌套组件 `useLoaderData` 空帧作用域缺陷；F4 `_telemetryHook` 每请求 set 竞态（若确有此用法）；L-4 未注册 tagName 静默空渲染无提示。
2. **建议（一致性/可维护性）**：H-2 统一 escape；M-1 删冗余层；M-3 统一 createLogger；M-4 拆分 `_handleClick`；F3 `_warned` 进程级单例。
3. **后续（清洁）**：M-5 去 default export（11 处）；L-1 locale 类型；L-2 清 3 处 Fragment 冗余；L-3 create test task；M-2 补单包 task（非 CI 必需）。

---

_本记录由团队主理人汇总，三方独立结论为 source of truth；未修改任何源码。_
Loading
Loading