Skip to content

test: gate UI releases on exact-core Dialog consumers - #157

Merged
elonehoo merged 3 commits into
mainfrom
fix/ui-151-core-compat-consumers
Oct 21, 2026
Merged

elonehoo merged 3 commits into
mainfrom
fix/ui-151-core-compat-consumers

Conversation

@elonehoo

@elonehoo elonehoo commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Refs #151. This adds an installed-artifact regression gate and a release-closure checklist, not another controlled-state migration. Main already contains the migration; published beta.17 does not change when main is fixed.

  • Build/pack each framework sequentially and consume the tarballs in isolated npm projects.
  • Pin core to exactly 0.2.13 and framework runtimes to exact versions, validate the installed core tree, retain lockfiles and SHA-256/version evidence.
  • Exercise Dialog with omitted open, defaultOpen=false, defaultOpen=true, and controlled open=false, including repeated open/close and callbacks.
  • Add the gate to Node 22.22.3 CI and before release publication, preserving existing gates.
  • Document the outstanding release/version/dist-tag/deprecation checks. Node >=22.22.3 stays unchanged.

Actual reproduction

With explicitly installed UI 0.0.1-beta.17 and core 0.2.13:

Framework Observed result
Vue 3.5.44 Uncontrolled Dialog with omitted open does not open
Solid 1.9.17 Uncontrolled Dialog with omitted open does not open
Svelte 5.57.2 Uncontrolled Dialog with omitted open does not open
React 19.3.0 Public package entry fails Vite resolution before mounting

The React result is a packaging failure, not evidence of a reproduced React state-management failure. The registry reproduction intentionally exits nonzero. The runner records build and interaction failures separately.

Validation

Executed locally on Node 24.19.0:

  • Four current-main packages built and packed successfully.
  • pnpm run test:core-compat: all four frameworks passed all four Dialog cases (16 cases), using installed tarballs and core 0.2.13.
  • Published beta.17 exact-version reproduction confirmed the results above; lockfiles and version records retained.
  • Full ESLint: 0 errors, 667 warnings in existing repository files; focused lint clean.
  • Publish safety tests: 383 passed; 14 runner regressions passed after review hardening (including symlinked temporary paths, preserving existing evidence, and cleanup on success/failure). The 16 packed Dialog cases were rerun through a symlinked evidence path and passed.
  • Latest review hardening records each UI artifact's resolved source and integrity in version evidence, verified against all four tarballs, and validates the workspace Node policy before installation. All 16 packed Dialog cases passed again. Exact callback assertions remain intentional: these fixtures dispatch one action at a time and there is no demonstrated legitimate duplicate callback.
  • Offline publish-scope check passed: 9 workspaces / 4 public targets.
  • git diff --check passed; independent focused code review found no blocker.

This Happy DOM consumer check is not real-browser accessibility, SSR, every-component, or every-framework-version coverage. All checks on commit da03ac3e9a3990757c509b920f1d41df8550fc54 passed, including Node 22.22.3/24, four framework packed-consumer jobs, browser accessibility smoke, lint, and the exact-core gate. CI results and consumer evidence. An initial parallel rebuild exited 129 in Solid; sequential builds and the final end-to-end gate were rerun successfully.

No npm publication, version bump, dist-tag change, deprecation, deployment, or issue closure is included. Keep #151 open until an authorized new release is verified from the registry.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Note

Low Risk
Changes are limited to CI/release gates and test scripts; no production package or auth/data-path changes, though a failing or flaky gate could block merges and releases.

Overview
Adds a release/CI gate that installs packed @destyler-ui/* tarballs (not workspace links) in isolated npm consumers, pins core 0.2.13 and fixed framework runtimes, and asserts Dialog open-state behavior across React, Vue, Solid, and Svelte via Vite + Happy DOM fixtures.

Introduces pnpm test:core-compat (unit tests for the harness plus build/pack + consumer runs), retains lockfiles/logs/results.json as evidence, and wires the check into CI (Node 22.22.3) and release workflows with artifact upload. Documents the #151 release-closure checklist and ignores generated test/core-compat/ output.

This is test and pipeline infrastructure only; it does not change library runtime code or publish versions.

Reviewed by Cursor Bugbot for commit da03ac3. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel

vercel Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
destyler-ui Ready Ready Preview Oct 21, 2026 8:06am UTC
destyler-ui-dev Ready Ready Preview Oct 21, 2026 8:06am UTC

@elonehoo
elonehoo marked this pull request as ready for review October 21, 2026 06:49

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c0f134b. Configure here.

Comment thread scripts/test-core-compat.mjs Outdated
@elonehoo

elonehoo commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

🤖 Review(中文)

更新于:edcf098(重新审查了完整 diff)

总结: 这个 PR 为 #151 增加了“安装产物”回归门禁:逐个框架 build + pack,在隔离的 npm 项目中安装 tarball,将 core 固定为 0.2.13,并在 Happy DOM 中测试 Dialog 的四种状态模式。新提交 edcf098 把公共逻辑抽到 scripts/core-compat-utils.mjs,并新增了 scripts/test-core-compat-runner.mjs 单元测试(已接入 test:core-compat)。实现与描述一致,没有改动库源码,CI 全部通过,风险较低。

上一轮建议的处理情况:

  • ✅ 1. 证据目录复用:prepareEvidenceDirectory 会在开始前检查目录是否为空并给出明确报错,且有测试覆盖。
  • ✅ 2. test/core-compat/ 已加入 .gitignore(antfu ESLint 配置默认会遵循 .gitignore)。
  • ✅ 3. assertCoreVersions 的报错信息现在会提示检查 overrides 和传递依赖。
  • ✅ 4. engines.node 改为与工作区 package.json 对比。
  • ✅ 5. requireTarball 会在安装前检查 tarball 是否存在且是文件。
  • ✅ 6. tarball 临时目录由 withTemporaryDirectory 在 finally 中清理,成功/失败两种情况都有测试。
  • ✅ 7. release.yml 中门禁已拆成独立 step。
  • ⏳ 8. 未处理:scripts/fixtures/core-compat/run.mjs 受控模式仍断言 changes 严格等于 [true]。

新的建议:

  1. scripts/fixtures/core-compat/run.mjs(沿用第 8 条):如果实现合法地多触发一次 onOpenChange(true),会造成误报。建议只断言“至少请求了一次 true、从未出现 false、Dialog 保持关闭”。非受控分支对回调序列的严格断言也有同样的风险,可以视情况放宽。
  2. scripts/test-core-compat-packed.mjs:tarball 现在会被删除,而保留下来的 lockfile 里记录的是已删除的 file: 路径(README 也说明了)。为了让证据能对应到具体产物,建议在 versions.json 中显式记录每个 tarball 的 SHA-256(或 lockfile 中的 integrity),而不只是 lockfile 本身的哈希。
  3. scripts/test-core-compat.mjs 第 643 行:workspace.engines.node 若工作区以后去掉 engines 会抛出 TypeError,并被记为 install 阶段失败,不太直观。可以在脚本开头断言 workspace.engines?.node 存在并给出明确信息(小问题)。

结论: 可以合并。上一轮的主要问题都已解决,剩下的都是小的健壮性改进,优先考虑第 1 条。

@elonehoo

elonehoo commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

🤖 Review (English)

Updated: edcf098 (full diff re-reviewed)

Summary: This PR adds an installed-artifact regression gate for #151: each framework is built and packed one at a time, the tarballs are installed into isolated npm projects with core pinned to 0.2.13, and four Dialog state modes are exercised in Happy DOM. The new commit edcf098 moves shared logic into scripts/core-compat-utils.mjs and adds unit tests in scripts/test-core-compat-runner.mjs, which now run as part of test:core-compat. The implementation matches the description, touches no library source, and CI is green, so the risk is low.

Status of earlier suggestions:

  • ✅ 1. Evidence reuse: prepareEvidenceDirectory checks up front that the directory is empty and fails with a clear message, with tests.
  • ✅ 2. test/core-compat/ is now in .gitignore (the antfu ESLint config respects .gitignore by default).
  • ✅ 3. The assertCoreVersions message now points at overrides and transitive dependencies.
  • ✅ 4. engines.node is compared against the workspace package.json.
  • ✅ 5. requireTarball checks that the tarball exists and is a file before installing.
  • ✅ 6. The tarball temp directory is removed in a finally by withTemporaryDirectory, tested for both success and failure.
  • ✅ 7. The gate is its own step in release.yml.
  • ⏳ 8. Not addressed: controlled mode in scripts/fixtures/core-compat/run.mjs still asserts changes is exactly [true].

New suggestions:

  1. scripts/fixtures/core-compat/run.mjs (carried over from 8): an implementation that legitimately fires onOpenChange(true) one extra time would fail the gate. Consider asserting only that true was requested at least once, false never appeared, and the Dialog stayed closed. The exact callback sequences in the uncontrolled branches carry the same risk and could be relaxed too.
  2. scripts/test-core-compat-packed.mjs: tarballs are now deleted, so the retained lockfiles point at file: paths that no longer exist (the README notes this). To tie the evidence to concrete artifacts, record each tarball's SHA-256 (or its lockfile integrity) explicitly in versions.json, not just the hash of the lockfile.
  3. scripts/test-core-compat.mjs line 643: if the workspace ever drops engines, workspace.engines.node throws a TypeError that gets recorded as an install-stage failure, which is confusing. Assert workspace.engines?.node at startup with a clear message (minor).

Verdict: Fine to merge. The main points from the last round are resolved; what's left is small robustness work, with 1 the most worthwhile.

@elonehoo

elonehoo commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

🤖 レビュー(日本語)

更新: edcf098(差分全体を再レビュー)

概要: この PR は #151 向けに「インストール済み成果物」の回帰ゲートを追加します。各フレームワークを順番に build・pack し、tarball を独立した npm プロジェクトにインストールして core を 0.2.13 に固定し、Happy DOM 上で Dialog の 4 つの状態モードを検証します。新しいコミット edcf098 では共通ロジックが scripts/core-compat-utils.mjs に切り出され、scripts/test-core-compat-runner.mjs のユニットテストが追加されて test:core-compat に組み込まれました。実装は説明と一致しており、ライブラリのソースは変更されておらず、CI もすべて通過しているため、リスクは低いです。

前回の指摘への対応状況:

  • ✅ 1. 証拠ディレクトリの再利用: prepareEvidenceDirectory が開始前に空であることを確認し、分かりやすいエラーを出すようになりました(テストあり)。
  • ✅ 2. test/core-compat/ が .gitignore に追加されました(antfu の ESLint 設定はデフォルトで .gitignore を尊重します)。
  • ✅ 3. assertCoreVersions のエラーメッセージで overrides と推移的依存の確認を促すようになりました。
  • ✅ 4. engines.node はワークスペースの package.json と比較するようになりました。
  • ✅ 5. requireTarball がインストール前に tarball の存在とファイルであることを確認します。
  • ✅ 6. tarball の一時ディレクトリは withTemporaryDirectory により finally で削除され、成功・失敗の両方がテストされています。
  • ✅ 7. release.yml でゲートが独立したステップになりました。
  • ⏳ 8. 未対応: scripts/fixtures/core-compat/run.mjs の制御モードは依然として changes が厳密に [true] であることを検証しています。

新しい提案:

  1. scripts/fixtures/core-compat/run.mjs(8 の継続): 実装が正当な理由で onOpenChange(true) を 1 回多く呼ぶと誤検知になります。「true が少なくとも 1 回要求された」「false は一度も出ていない」「Dialog は閉じたまま」だけを検証することを検討してください。非制御モードのコールバック列の厳密な検証にも同じリスクがあるので、必要に応じて緩めてもよいと思います。
  2. scripts/test-core-compat-packed.mjs: tarball が削除されるようになったため、保存される lockfile は存在しない file: パスを指すことになります(README にも記載あり)。証拠を具体的な成果物と結び付けるため、lockfile 自体のハッシュだけでなく、各 tarball の SHA-256(または lockfile の integrity)を versions.json に明示的に記録することをおすすめします。
  3. scripts/test-core-compat.mjs 643 行目: 将来ワークスペースから engines が削除されると、workspace.engines.node が TypeError を投げ、install 段階の失敗として記録されて分かりにくくなります。起動時に workspace.engines?.node の存在を明確なメッセージ付きで検証するとよいです(軽微)。

結論: マージして問題ありません。前回の主な指摘は解決済みで、残りは小さな堅牢性の改善です。優先するなら 1 です。

@elonehoo
elonehoo marked this pull request as draft October 21, 2026 07:05
@elonehoo
elonehoo marked this pull request as ready for review October 21, 2026 07:31
@elonehoo
elonehoo marked this pull request as draft October 21, 2026 07:49
@elonehoo
elonehoo marked this pull request as ready for review October 21, 2026 08:15
@elonehoo
elonehoo merged commit 0fc073c into main Oct 21, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant