Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose event.routeId and page.routeId #4345

Merged
merged 7 commits into from
Mar 16, 2022
Merged

expose event.routeId and page.routeId #4345

merged 7 commits into from
Mar 16, 2022

Conversation

Rich-Harris
Copy link
Member

closes #3840 (and #3907) by exposing the internal route.key as event.routeKey. This is a string based on the filename of the page/endpoint:

filename event.routeKey
src/routes/index.svelte ""
src/routes/about.svelte "about"
src/routes/blog/index.js "blog"
src/routes/blog/index.svelte "blog"
src/routes/blog/[slug].svelte "blog/[slug]"
src/routes/blog/archive/[page=integer]/index.svelte "blog/archive/[page=integer]"
src/routes/[...catchall] "[...catchall]"

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:/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 pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Mar 16, 2022

🦋 Changeset detected

Latest commit: d25b687

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

@Rich-Harris Rich-Harris changed the title expose event.routeKey - expose event.routeKey Mar 16, 2022
@Conduitry
Copy link
Member

Do we want to expose this in $page as well? Or are you thinking that, if someone really wanted this on the client, they could expose it in $session manually?

@Rich-Harris
Copy link
Member Author

We definitely could put it in page (one of the changes in this series of PRs makes all page keys available to the client as a side effect), I was just being as conservative as possible. Do you have a use for it?

@Conduitry
Copy link
Member

It was one of my original ideas in #269 before that turned into exposing $page.stuff. When doing navbar-type things, there are still times where routeKey would be less work than manually having to set different stuff values in each route (which is probably only made more so with shadow endpoints, if the only reason you'd need a particular load is to set stuff).

@Rich-Harris
Copy link
Member Author

In that case is page.routeKey the right property, or should it just be event.key and page.key?

@Rich-Harris
Copy link
Member Author

For posterity: we decided on event.routeId and page.routeId

@Rich-Harris
Copy link
Member Author

merging this into #4344 to try and minimise #4354-style git fuckups

@Rich-Harris Rich-Harris merged commit 1a2e432 into gh-1194 Mar 16, 2022
@Rich-Harris Rich-Harris deleted the gh-3840 branch March 16, 2022 16:15
Rich-Harris added a commit that referenced this pull request Mar 16, 2022
* remove fallthrough

* changeset

* remove fallthrough documentation

* tweak docs

* simplify

* simplify

* simplify a tiny bit

* add failing test of param validators

* client-side route parsing

* tidy up

* add validators to manifest data

* client-side validation

* simplify

* server-side param validation

* lint

* oops

* clarify

* docs

* minor fixes

* fixes

* ease debugging

* vanquish SPA reloading bug

* simplify

* lint

* windows fix

* changeset

* match route before calling handle - closes #1194

* changeset

* throw error if validator module is missing a validate export

* update configuration.md

* Update documentation/docs/01-routing.md

* tighten up validator naming requirements

* disallow $ in both param names and types

* changeset

* point fallthrough users at validation docs

* add some JSDoc commentsd

* expose `event.routeId` and `page.routeId` (#4345)

* expose event.routeKey - closes #3840

* change routeKey to routeId

* rename routeKey to routeId, expose page.routeId

* rename route.key -> route.id everywhere

* adapter-node sure picked a weird time to stop typechecking

* oops

* Update .changeset/mean-crews-unite.md
rmunn added a commit to rmunn/kit that referenced this pull request Apr 7, 2022
Because of how sveltejs#4345 was merged into sveltejs#4344, the script that automatically
adds PR numbers to the changelog got the wrong PR number for the "Expose
`event.routeId` and `page.routeId`" feature. This manual change to 
the CHANGELOG.md file fixes that minor error.
ignatiusmb added a commit that referenced this pull request Apr 7, 2022
* Update one incorrect PR number in changelog

Because of how #4345 was merged into #4344, the script that automatically
adds PR numbers to the changelog got the wrong PR number for the "Expose
`event.routeId` and `page.routeId`" feature. This manual change to 
the CHANGELOG.md file fixes that minor error.

* Update packages/kit/CHANGELOG.md

Fix link destination as well as link text

Co-authored-by: Ignatius Bagus <[email protected]>

Co-authored-by: Ignatius Bagus <[email protected]>
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.

2 participants