fix(qwik-router): submit() always rejects when the submission aborts#8729
Draft
maiieul wants to merge 27 commits into
Draft
fix(qwik-router): submit() always rejects when the submission aborts#8729maiieul wants to merge 27 commits into
maiieul wants to merge 27 commits into
Conversation
…`fail()` Route loaders and actions now report failures via a typed `.error` (a `ServerError`) instead of `fail()`/`value.failed`. `throw error(...)` and failed validators surface on `loader.error`/`action.error` consistently on server and client; `.value` is the success type only. `ServerError` exposes its payload fields directly (`error.fieldErrors`), typed from the validator schema, with `.data` kept as the canonical payload. Same-origin absolute `rewrite()` now normalizes to a relative rewrite instead of erroring. BREAKING CHANGE: `requestEvent.fail()`, the `FailReturn` type, and `value.failed` are removed. Throw `error(status, data)` and read `loader.error`/`action.error` instead.
…result requestEv.fail(status, data) returns a FailReturn<T> branded with a unique symbol (non-enumerable, JSON-unspoofable, unlike v1's failed:true check). Constructor overloads split the return union: .value = ExcludeFail<OBJ>, .error = ServerError<FailPayload<OBJ> | validator errors>. Pure producer: status applies at conversion time, so an unreturned fail() is a no-op. Runtime conversion to the .error state lands in the next commit.
…to abort semantics
One conversion choke point (failToServerError + applyFailureResponse):
validator failures and returned fail() results become the loader/action
.error state with status + Cache-Control hygiene; thrown error() and
unexpected errors propagate to middleware again (v1 abort semantics).
Multi-loader failure status is deterministic (first in registration
order). The q-loader endpoint keeps its 200 {e} envelope but never
caches failures. resolveValue rejects when the depended-on loader
failed. Failed loaders are memoized like successes.
… error types
Thrown error() on JSON paths now ships an abort envelope: loaders fall
back to a full-page load (server renders the real error page; GETs are
safe to replay), actions reject run()/submit() and record no state —
<Form> surfaces aborts via the submitcompleted detail instead (avoids
unhandled rejections). Fixes the {e,s}-vs-{error} envelope mismatch that
silently swallowed middleware errors. ActionReturn/FormSubmitCompletedDetail
gain error (value now optional); LoaderSignal.error is honestly typed
ServerError<ERROR> | Error (client transport failures); isServerError()
narrows structurally so it works across serialization boundaries.
RewriteMessage carries search separately so it can't be percent-encoded
into the pathname. An explicit query on the rewrite target replaces the
request's query; otherwise the original is kept. Fragments are dropped
(they never reach the server). Absolute-URL detection now requires a
protocol instead of startsWith('http'), which matched relative paths
like /http-docs.
…erate API docs Action/route-loader/validator pages rewritten for return-fail()-to-.error plus throw-error()-aborts; error-handling and complex-forms pages fixed (were teaching the removed model); v1→v2 upgrade section added. Changesets rewritten. api.update run; ae-forgotten-export warnings fixed by exporting FailOfRest/ValidatorReturn*/ServerError from the runtime entry (removes the machine-absolute path from the checked-in api.md).
Fixtures use return fail() for expected failures (typed .error, inline UI, status + no Cache-Control) and throw error() for intentional error pages (plugin@errors interception restored). New loader-fail fixture and specs: inline 429 rendering, SPA full-page fallback on thrown loader errors. Unit tests added for rewrite() branches (8) and multi-loader failure status determinism (2).
The same-origin absolute URL normalization and query/fragment handling for rewrite() are orthogonal to the failure-model redesign — extracted to their own PR. rewrite() keeps the build/v2 behavior here (absolute URLs are rejected with a 400).
…re unions action.error?.fieldErrors typechecks without narrowing when validator and fail() payloads mix; missing keys are typed ?: never (v1's .value ergonomics, now on .error).
Restores the v1 capability of reading failed action/loader state in document head functions: ResolveSyncValue is typed Awaited<T> | ServerError<StrictUnion<ERROR>> (| undefined for actions) and failed loaders return their error instead of throwing 'Loader not executed'. Narrow with isServerError().
Plain property checks discriminate success vs failure in head() — no isServerError needed.
The transport-failure Error member carries the server failure's fields as ?: never, so loader.error?.status / payload fields typecheck and discriminate without isServerError. Guard demoted to catch-block edges in the docs.
…e level Type pins: truthy status narrows to the ServerError member (payload non-optional inside the branch). New SPA e2e: an aborted q-loader fetch lands a plain Error on loader.error and the non-status branch renders.
…n state, type collapses q-loader failure envelopes no longer get max-age re-applied after the hygiene delete; abort envelopes scrub Cache-Control. Aborted/bailed submissions reset isNavigating; non-JSON action responses settle as aborts instead of hanging submit() forever. Aborts record no state (status included). PE POSTs keep the action's failure status over a loader's. LoaderSignal no longer collapses to never for always-failing loaders; head() resolveValue returns loader errors instead of throwing; StrictUnion restored on .value; ServerError exported as a value from the runtime entry; TransportError exported (api.md path leak gone). New tests: SPA-nav fail() envelope e2e (status 200, uncached, reactive 429), real-path resolveValue rejection, never-collapse pin.
…rError flattening Aborts (thrown error() / unexpected server errors) now reject the submit()/run() promise unconditionally — the client mirror of the server-side throw. <Form> catches its own invocation's rejection and keeps surfacing aborts via submitcompleted detail.aborted. ServerError no longer flattens reserved payload keys: error.message is always a string, payloads can't spoof status/data or replace the instance prototype, and the flat type mirrors this with Omit. login fixture gates the generic error block on the typed union instead of message falsiness. New abort e2e fixture + spec, server-error unit tests.
… submit-rejection contract isServerError (minor) and always-reject-on-abort (patch) move to follow-up PRs for a separate release. Interim abort contract: a programmatic submit() rejects; <Form> surfaces aborts via submitcompleted detail.aborted. The ServerError reserved-key hardening stays (patch changeset).
🦋 Changeset detectedLatest commit: 7062395 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
maiieul
force-pushed
the
v2-submit-rejection
branch
from
555e477 to
be3581f
Compare
@qwik.dev/core
@qwik.dev/router
eslint-plugin-qwik
create-qwik
@qwik.dev/optimizer
commit: |
maiieul
force-pushed
the
v2-submit-rejection
branch
from
be3581f to
82f1250
Compare
Contributor
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
maiieul
force-pushed
the
v2-submit-rejection
branch
from
82f1250 to
52467bd
Compare
…ssions detail.aborted moves to a follow-up minor; the abort delivery plumbing (envelope, rejection for programmatic submit, state reset) stays as the fix for aborts silently resolving as successes.
maiieul
force-pushed
the
v2-submit-rejection
branch
from
52467bd to
2a1d691
Compare
…elope caching, value export move to follow-ups
…tcompleted carries detail.aborted Patch: thrown errors during SPA submissions were silently swallowed as empty successes; submit()/run() now rejects, isNavigating resets, and non-JSON responses settle. Minor: submitcompleted also fires on aborts with the error on detail.aborted.
maiieul
force-pushed
the
v2-submit-rejection
branch
from
2a1d691 to
7062395
Compare
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
1 participant
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.
What is it?
Description
Two changesets:
submit()/run()now always rejects when the submission aborts — previously aSubmitEvent-based submission resolved with an empty result, letting failures look like successes (patch). Andsubmitcompletednow also fires on aborted submissions, with the error ondetail.aborted, so<Form>users can observe aborts (minor). Stacked on #8717, for the release after the major ships.