fix: clone Date and RegExp values in structuredClone - #10975
proggeramlug wants to merge 2 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthrough
ChangesStructured clone object support
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Caller
participant structuredClone
participant js_structured_clone_inner
participant RuntimeObject
Caller->>structuredClone: clone Date or RegExp
structuredClone->>js_structured_clone_inner: process value
js_structured_clone_inner->>RuntimeObject: allocate independent clone
js_structured_clone_inner-->>structuredClone: return memoized clone
structuredClone-->>Caller: return clone
Merge Risk: ⚪ Minimal · up to structuredClone now creates independent Date and RegExp values with the expected copied state. The supplied regression coverage validates the behavior, so the change is ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Landed on main in merge train 256 (#11018, v0.5.1638), main Carried at head Trains rebase-merge, so commits get new SHAs and GitHub cannot mark this PR merged. Closed as landed. |
Summary
GC_TYPE_REGEXPlayout. The former branch underGC_TYPE_OBJECTcould no longer run, so RegExp clones also kept the source identity. A clone retains source and flags, starts withlastIndex = 0, and can be mutated independently.Closes #10829.
Validation
RUST_TEST_THREADS=1 CARGO_BUILD_JOBS=4 cargo test -p perry-runtime --lib structured_clone -- --test-threads=1(6 passed)cargo fmt --all --checkandgit diff --checktest-files/test_gap_10829_structured_clone_date_regexp.ts.Scope
The existing Map, Set, ArrayBuffer, typed buffer, and DataView branches already allocate clone objects. Error and Blob/File behavior needs separate work; this PR fixes the Date aliasing reported in #10829 and the adjacent dead RegExp clone branch.
Summary by CodeRabbit
structuredCloneso clonedDateandRegExpobjects are independent from their originals.lastIndexto match expected runtime behavior.