Skip to content

augment Navigation type - #6552

Merged
Rich-Harris merged 2 commits into
masterfrom
gh-6551
Sep 3, 2022
Merged

augment Navigation type#6552
Rich-Harris merged 2 commits into
masterfrom
gh-6551

Conversation

@Rich-Harris

Copy link
Copy Markdown
Member

closes #6551 — gives beforeNavigate and afterNavigate more information about which route they are navigating from and to

Please don't delete this checklist! 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. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

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

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot

changeset-bot Bot commented Sep 3, 2022

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b740e24

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

@netlify

netlify Bot commented Sep 3, 2022

Copy link
Copy Markdown

Deploy Preview for kit-demo ready!

Name Link
🔨 Latest commit b740e24
🔍 Latest deploy log https://app.netlify.com/sites/kit-demo/deploys/6313c2fac6d89f00089b2d2e
😎 Deploy Preview https://deploy-preview-6552--kit-demo.netlify.app/build
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Comment thread packages/kit/src/runtime/client/client.js Outdated
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
@Rich-Harris
Rich-Harris merged commit 82aed98 into master Sep 3, 2022
@Rich-Harris
Rich-Harris deleted the gh-6551 branch September 3, 2022 21:11
@github-actions github-actions Bot mentioned this pull request Sep 3, 2022
Rich-Harris pushed a commit that referenced this pull request Jul 22, 2026
…error page (#16381)

After an error page renders, `current.route` is `null`
(`load_root_error_page` stores `route: null` in the navigation state).
The next navigation computes

```js
const route_changed = current.route ? route.id !== current.route.id : false;
```

so the route counts as unchanged no matter where the user goes next. A
root layout server load that tracked `route` is then skipped as fresh,
and its data from before the error page renders on the new page. The new
test's failing assertion on `version-3` shows the layout still claiming
the previous route:

```
Expected: "/b"
Received: "/a"
```

A missing `current.route` now counts as changed. On a first-ever
navigation the flag is irrelevant, the node has no previous data to
reuse anyway, so the only behavior change is the conservative refetch
after error pages.

The interaction was never designed. `route: null` came in #6552 to give
`beforeNavigate` a value meaning "no matched route", and the guard came
two months later in #7450 with route tracking itself, mirroring the
adjacent `url_changed` null-guard. Neither discussed the error-page
case.

The test lives in the no-ssr app. Its root layout gains a server load
returning `route.id`, the navigation's data request is intercepted with
a 500 to produce the error page, and the follow-up navigation must show
the new route id. The app's root `+layout.server.js` was an empty file,
shipped by #11354 so its 404-loop test has a server load node in dev;
giving it a real load keeps that trigger intact.

Found while reviewing #16380, which mirrors this computation for the
error page itself and carries the same one-line fix.

---

### Please don't delete this checklist! Before submitting the PR, please
make sure you do the following:
- [x] It's really useful if your PR references an issue where it is
discussed ahead of time. In many cases, features are absent for a
reason. For large changes, please create an RFC:
https://github.com/sveltejs/rfcs
- [x] This message body should clearly illustrate what problems it
solves.
- [x] Ideally, include a test that fails without this PR but passes with
it.

### Tests
- [x] Run the tests with `pnpm test` and lint the project with `pnpm
lint` and `pnpm check`

### Changesets
- [x] If your PR makes a change that should be noted in one or more
packages' changelogs, generate a changeset by running `pnpm changeset`
and following the prompts. Changesets that add features should be
`minor` and those that fix bugs should be `patch`. Please prefix
changeset messages with `feat:`, `fix:`, or `chore:`.

### Edits

- [x] Please ensure that 'Allow edits from maintainers' is checked. PRs
without this option may be closed.
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.

More information in Navigation

2 participants