Repository navigation
ref(cli): Prefer Array.from() over spread to copy iterables - #1721
Open
sentry-junior[bot] wants to merge 2 commits into
Open
sentry-junior[bot] wants to merge 2 commits into
sentry-junior[bot] wants to merge 2 commits into
Conversation
Add the sentry-cli/prefer-array-from lint rule. It flags array literals whose only element is a spread, such as [...set], and autofixes them to Array.from(set). Spreads that build arrays from several parts, call arguments, and object spreads are left alone. Apply the autofix across CLI src and tests and document the convention in packages/cli/AGENTS.md. Co-Authored-By: BYK <burak.kaya@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a24f451. Configure here.
BYK
approved these changes
Oct 29, 2026
Contributor
Author
BYK
added a commit
that referenced
this pull request
Oct 29, 2026
Replaces #1721. That PR standardized the CLI on `Array.from(x)`. After looking into why the ecosystem defaults to spread, the evidence points the other way, so this enables Oxlint's built-in `unicorn/prefer-spread` at the root and applies it across the repo. ## Why spread - **It's stricter.** TypeScript rejects `[...x]` when `x` isn't iterable, and the runtime throws. `Array.from` accepts any `{ length }` object (it type-checks as `unknown[]`) and quietly returns `[]` for numbers and `ArrayBuffer`s. Sindre Sorhus gave this as the reason for the rule ([unicorn#120](sindresorhus/eslint-plugin-unicorn#120)): array-likes are a legacy quirk, and iterables are the real protocol. - **Performance is the same.** V8 7.2 added fast paths for spreading arrays, strings, Sets, and Map keys/values, and `Array.from(iterable)` reuses them ([v8.dev](https://v8.dev/blog/spread-elements)). On Node 22 here, the two are within noise for arrays, Sets, `map.values()`, and strings at 10, 1k, and 100k elements. Results for Map entries and generators are mixed, but small. - **The `Array.from(x, fn)` advantage doesn't show up.** Airbnb 4.6 recommends it to avoid an intermediate array. On Node 22, `Array.from(set, fn)` measured 2–4× *slower* than `[...set].map(fn)`, because the mapping form skips the fast path. The rule still allows it, but it buys nothing. - **One syntax for copying and combining.** `[...a]`, `[...a, ...b]`, and `[...path, key]` read as one family. #1721's rule left a split where a lone spread became `Array.from` but a multi-part spread stayed. - **Less to maintain.** The rule is built into Oxlint and already autofixes. There's no custom rule. The unicorn rule leaves alone the cases where `Array.from` is the right tool: `Array.from({ length: n }, fn)`, array-likes, and any call with a map function. ## What changed - `.oxlintrc.json`: `"unicorn/prefer-spread": "error"` in the root rules. - The autofix rewrote single-argument `Array.from(x)` and array `.slice()` copies across the CLI, MCP core, Cloudflare, docs-mcp, test client, and scripts (75 sites). The rest were fixed by hand: - **Array `.concat()`** (5 sites): now `[...a, ...b]` and `[...path, key]`. - **`String#concat` in `apps/docs-mcp` (false positive):** now a template literal. - **Strings:** turning `Array.from(str)` into `[...str]` trips the already-enabled `typescript/no-misused-spread`. The two forms behave the same at runtime, so the lint split is only syntax. I went by what each site means: - Code-unit checks (`charCodeAt` predicates in issue-linking, `ci-projects.mjs`, and the delta-upgrade test helpers) now use `split("")`. That's typescript-eslint's recommended form, and it matches the `charCodeAt` logic exactly. - Three places count code points on purpose: banner width, the pixel-canvas glyph check, and the help test. They keep the spread with a reasoned `oxlint-disable-next-line`. - **Uint8Array assertions** in `infrastructure.test.ts` use `subarray()`, so `[...]` still produces an Array. `no-useless-spread` assumes `.slice()` returns an Array. - The convention is documented in `docs/contributing/coding-guidelines.md`. ## Verification - `pnpm run lint` (oxlint, oxfmt, ast-grep): clean - `pnpm run tsc` (all packages): clean - Tests: - mcp-core: 1,885 passed - mcp-cloudflare: 443 passed - docs-mcp: 49 passed - mcp-test-client: 80 passed - `test:ci-projects`: 94 passed - CLI unit: 11,758 passed, 6 failed. The 6 are the same permission tests that fail on clean `main` in this sandbox (`completions`, `code-scanner`, `file-changes`, `telemetry`, `upgrade`). <!-- junior-request-attribution:start --> via **BYK**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:local%3Aweb%3A7fba2603229556152deec3d2 --> -- [View Junior Session in Sentry](https://sentry.sentry.io/explore/conversations/local%3Aweb%3A7fba2603229556152deec3d2/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: BYK <burak.kaya@sentry.io>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CLI copied iterables both ways: about 71
[...x]against 12Array.from(x)insrc/, and more in tests. No existing rule settles it. Oxlint'sunicorn/prefer-spreadpushes the other way and isn't enabled.This adds
sentry-cli/prefer-array-fromto the CLI Oxlint plugin. It flags an array literal whose only element is a spread ([...set],[...map.values()],[...(a ?? b)]) and autofixes it toArray.from(...). It leaves these alone:[...a, ...b],[...items, tail]fn(...args)The autofix is applied across
packages/cli/srcandpackages/cli/test(≈180 sites, 101 files).tscstays clean, so no tuple types were relied on. The convention is documented under "Copying Iterables" inpackages/cli/AGENTS.md.This is a consistency rule, not a performance fix.
Array.fromreads more clearly as a conversion, also accepts array-likes, and takes a map callback. Spread is just as correct for iterables.Verification
node --test scripts/cli-lint-rules.test.mjs: 57 passed, including new valid and invalid cases with fix outputpnpm run lint(oxlint, oxfmt, ast-grep): cleantsc --noEmitinpackages/cli: cleanmainin this sandbox. They are all permission tests (completions,code-scanner,file-changes,telemetry,upgrade), so they're environment failures and not caused by this diff.via BYK.
--
View Junior Session in Sentry