Skip to content

Store only key-value pairs in Hash arrays - #1341

Open
OskarEichler wants to merge 2 commits into
basecamp:mainfrom
OskarEichler:codex/hash-to-array-pairs
Open

Store only key-value pairs in Hash arrays#1341
OskarEichler wants to merge 2 commits into
basecamp:mainfrom
OskarEichler:codex/hash-to-array-pairs

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

Remove the nested result.push in Hash#toArray.

The inner push already appends the key and value; pushing its numeric return value again inserts cumulative array lengths. The corrected method produces the intended flat key/value sequence while preserving the serialized JSON output.

The generated Action Text asset is rebuilt with the same one-line change.

Verification

  • focused model: ["a", 1, 2, "b", 2, 5] becomes ["a", 1, "b", 2]; JSON remains identical
  • complete Chromium suite: 501 total, 472 pass, 0 fail, 29 conditional skips
  • frozen Yarn install, ESLint, Rollup/Sass/assets and gem build pass
  • Action Text wrapper test and Rails loading pass on Ruby 4.0.6 and Ruby 3.2.11

No public API or dependency change.

Copilot AI balanced review requested due to automatic review settings August 30, 2026 02:23

Copilot AI 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.

Pull request overview

Fixes Hash#toArray so it returns only flattened key-value pairs and keeps the packaged Action Text asset synchronized.

Changes:

  • Removes the nested push that inserted array lengths.
  • Rebuilds the generated Action Text bundle.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/trix/core/collections/hash.js Corrects Hash#toArray.
action_text-trix/app/assets/javascripts/trix.js Synchronizes the generated asset.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

for (const key in this.values) {
const value = this.values[key]
result.push(result.push(key, value))
result.push(key, value)
@OskarEichler

Copy link
Copy Markdown
Author

Added the requested exact regression assertion for Hash({ a: 1, b: 2 }).toArray(). ESLint passes and the JavaScript and test bundles build successfully.

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.

2 participants