chore: keep the call context on the event view instead of cloning the request state

[Nic-Polumeyv]

RequestState carries five is_in_* booleans that say what kind of code is on the stack. Flipping one means cloning the whole state and re-entering the store with the copy, and every other copy of the event (merge_tracing, the resolve wrapper, sequence) is a spread that carries whatever the source happens to enumerate.

The event is now a class in packages/kit/src/exports/internal/server/event.js, and event.clone(kind) is the only way to copy it: a view for a kind of code with a fixed field list, the kinds on the stack as bit flags read through getters like event.in_query, and a QueryEvent whose url, params and route throw on access. In dev, with_request_store refuses anything that is not a RequestEvent, so a stray spread fails at the store instead of silently losing its restrictions.

Fixes #17035. requested() reads the kind set by the command/form wrapper, so it works wherever the command is called from.

[pkg-svelte-dev]

Install the latest version of @sveltejs/kit from e0cadf4:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/e0cadf43e453b5150f90fd9ddf3a7bd0ff52e5e5

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/16968

[changeset-bot]

🦋 Changeset detected

Latest commit: e0cadf4

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

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	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

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-kit-16968-svelte.vercel.app/

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The change replaces transient remote and render state flags with derived RequestEvent contexts. It adds context-aware cookie, header, query, mutation, and render handling. Remote functions now receive explicit operation identifiers. Request state now tracks response headers and completion. Rendering, data loading, error handling, and response creation use cloned events. Internal type references and test fixtures now use the internal RequestEvent implementation.

Merge Risk: 🟡 Moderate · up to dfdbd

Direct remote forms that invoke requested-query operations will fail instead of executing. This breaks a supported form workflow and should be fixed before merge. Development also emits a spurious warning for valid Cache-Control configuration.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Backward Compatibility Impact Disclosure	⚠️ Warning	The pull request introduces breaking exported type changes. RequestState adds required headers and responded fields, removes five is_in_* fields, and changes RequestStore.event to the intern…	Add a changeset for @sveltejs/kit with major in its front matter. Start the description with breaking: and document the removed RequestState fields, the new required fields, and the RequestStore.event type change.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title uses the required chore: prefix and accurately describes the main change: preserving call context on event views instead of cloning request state.

Full details: Backward Compatibility Impact Disclosure

Explanation

The pull request introduces breaking exported type changes. RequestState adds required headers and responded fields, removes five is_in_* fields, and changes RequestStore.event to the internal RequestEvent class. These types are exported through the package's internal entry points. The authoritative diff contains no .changeset file, and no existing changeset documents this change.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[elliott-with-the-longest-name-on-github]

This looks overall good, but I'm slightly concerned about the fact that we're putting the query restrictions on a prototype object, because prototypes aren't preserved through spreads.

The restrictions are applied through this:

const derived = Object.assign(Object.create(QUERY_PROTOTYPE), event, ...);

...but in merge_tracing we spread the event:

export function merge_tracing(event_like, current) {
	return {
		...event_like,
		tracing: {
			...event_like.tracing,
			current
		}
	};
}

Object spread copies the enumerable properties, but does not preserve the prototype. Therefore:

const query_event = derive_event(event, 'query');
const traced_event = merge_tracing(query_event, span);

...produces a normal object, where inside(traced_event, 'query') === true, but the protections for traced_event.url, .params, and .route are gone. I don't think we're actually doing this right now (so it's probably a latent bug). I'm not sure what exactly the best solution for that is... 🤔

[Nic-Polumeyv]

Solid catch. v3 has the same hole (not 100% sure if you were alluding to already knowing that). But... the non-enumerable getters this replaces are skipped by that spread too, and either way it gives url === undefined, not the real URL. So this PR doesn't loosen anything.

I'm thinking of just routing merge_tracing through derive_event(event, null, { tracing })... that should fix it?

Edit: Actually, rather than patching merge_tracing, we could make the guard a value instead of an accessor. url, params and route on a query view become three module-level proxies that throw on any use, assigned as ordinary data properties:

const poison = (property) => {
      const fail = () => {
              throw new Error(`Cannot access event.${property} in a query. Pass the value as an argument to the query instead`);
      };
      return new Proxy(Object.freeze({}), { get: fail, has: fail, ownKeys: fail, set: fail });
};

const POISON = { url: poison('url'), params: poison('params'), route: poison('route') };

Every copy mechanism carries data properties, and derive_event collapses to a plain literal:

return {
      ...event,
      ...overrides,
      ...(entering & KINDS.remote ? restrictions(event, flags) : null),
      ...(flags & KINDS.query ? POISON : null),
      [CONTEXT]: flags
};

That also drops Object.create and Object.assign. Amazingly, it's about 3x faster per view on V8. The one behaviour change is that the throw moves from touching event.url to using it, so typeof event.url no longer throws.

[elliott-with-the-longest-name-on-github]

Hmm, I'm not sure I love that the error is moving from event.url to "doing something with event.url"... it makes it easier to run into something like this:

const url = getRequestEvent();
if (something) {
  url.pathname // throws
}

which just means there are more opportunities for you to write code that throws sometimes but not all the time. As much as I don't like the additional complexity it brings, I think using derive_event inside of merge_tracing is probably the better call. We could also define the CONTEXT property as non-enumerable so that if we ever spread the event somewhere, it's more immediately obvious that we've screwed up (because it's more likely we run into "CONTEXT no longer exists on this object"-related errors).

[Nic-Polumeyv]

Ohh ok. I underweighted that. Going with my first option then

[coderabbitai]

Actionable comments posted: 1

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)

packages/kit/src/exports/internal/server/validate-headers.js-8-8 (1)

8-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Accept must-understand in VALID_CACHE_CONTROL_DIRECTIVES.

validateHeaders currently warns for this IANA-registered Cache-Control directive in development. Add it beside must-revalidate.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: QUIET

Plan: Advanced

Run ID: d69ea76a-c12a-443b-9ed9-781b21192a25

📥 Commits

Reviewing files that changed from the base of the PR and between 7e364fd and dfdbdf6.

📒 Files selected for processing (29)

• packages/kit/src/exports/hooks/sequence.spec.js
• packages/kit/src/exports/internal/server/event.js
• packages/kit/src/exports/internal/server/event.spec.js
• packages/kit/src/exports/internal/server/index.js
• packages/kit/src/exports/internal/server/validate-headers.js
• packages/kit/src/exports/internal/server/validate-headers.spec.js
• packages/kit/src/runtime/app/server/remote/command.js
• packages/kit/src/runtime/app/server/remote/form.js
• packages/kit/src/runtime/app/server/remote/prerender.js
• packages/kit/src/runtime/app/server/remote/prerender.spec.js
• packages/kit/src/runtime/app/server/remote/query.js
• packages/kit/src/runtime/app/server/remote/requested.js
• packages/kit/src/runtime/app/server/remote/shared.js
• packages/kit/src/runtime/server/data/index.js
• packages/kit/src/runtime/server/endpoint.js
• packages/kit/src/runtime/server/errors.js
• packages/kit/src/runtime/server/fetch.js
• packages/kit/src/runtime/server/page/actions.js
• packages/kit/src/runtime/server/page/data_serializer.js
• packages/kit/src/runtime/server/page/index.js
• packages/kit/src/runtime/server/page/load_data.js
• packages/kit/src/runtime/server/page/render.js
• packages/kit/src/runtime/server/page/respond_with_error.js
• packages/kit/src/runtime/server/remote-functions.js
• packages/kit/src/runtime/server/remote-functions.spec.js
• packages/kit/src/runtime/server/respond.js
• packages/kit/src/runtime/server/state.js
• packages/kit/src/runtime/server/utils.js
• packages/kit/src/types/internal.d.ts

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• sveltejs/vite-plugin-svelte (manual)
• vitejs/vite (manual)
• sveltejs/svelte (manual)

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[elliott-with-the-longest-name-on-github]

Just some thoughts... I need to think about this approach some more before I can decide if it's worth it. Will come back to it tomorrow