Skip to content

perf: use Set for exclude checks in rest_props, exclude_from_object, legacy_rest_props - #18654

Open
Xsidz wants to merge 1 commit into
sveltejs:mainfrom
Xsidz:fix/rest-props-set-membership
Open

perf: use Set for exclude checks in rest_props, exclude_from_object, legacy_rest_props#18654
Xsidz wants to merge 1 commit into
sveltejs:mainfrom
Xsidz:fix/rest-props-set-membership

Conversation

@Xsidz

@Xsidz Xsidz commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time.
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Fixes #18601

What changed

Three sibling paths were using Array.includes() (O(n)) for exclude-list membership checks, while the modern client runes rest_props already used a Set. This PR brings the remaining paths in line.

Path File Change
Server SSR rest_props packages/svelte/src/internal/server/index.js new Set(rest) at entry, .has()
exclude_from_object packages/svelte/src/internal/shared/utils.js new Set(keys) at entry, .has()
legacy_rest_props_handler packages/svelte/src/internal/client/reactivity/props.js new Set(exclude) at construction, .has() / .add()

No signature changes — callers (including the compiler) still pass arrays. Conversion to Set is done internally. The only behavioral difference: deleteProperty on legacy_rest_props_handler now calls .add() instead of .push().

Test plan

  • Full test suite: 7595 tests pass, 0 new failures
  • Pre-existing browser suite failure (Playwright not installed) is unrelated
  • git diff HEAD reviewed — only intended files changed

…gacy_rest_props

Replaces Array.includes() (O(n)) with Set.has() (O(1)) across three
sibling paths that were missed when client runes rest_props was updated:
- server/index.js:rest_props (called per SSR request)
- shared/utils.js:exclude_from_object (called for rest destructuring)
- client/reactivity/props.js:legacy_rest_props_handler ($$restProps proxy)

No signature changes — arrays are still accepted and converted to Set
internally. legacy_rest_props.deleteProperty push() → add() to match.

Fixes sveltejs#18601
Copilot AI lite review requested due to automatic review settings August 16, 2026 17:25
@pkg-svelte-dev

Copy link
Copy Markdown

Install the latest version of svelte from 8103d62:

pnpm add https://pkg.svelte.dev/svelte/c/8103d62b404578f4922e4fb2a8a981673418c831

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/svelte/pr/18654

Note

This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed.

@changeset-bot

changeset-bot Bot commented Aug 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8103d62

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@dummdidumm dummdidumm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I doubt this will speed up things - it's more likely to slow it down, since this first creates a Set out of the array. A change like this needs to more like #18252 where the compiler was also adjusted to create a hoisted Set. If that is not possible then I think it's better to not do this at all.

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.

perf: use Set for rest-prop exclude lists (server, legacy, exclude_from_object)

3 participants