-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: expose event.route
and event.url
to remote functions
#14606
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
Conversation
🦋 Changeset detectedLatest commit: 1edecc4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Co-authored-by: Patrick <Patrick@ShowYou.us>
Should we add a note in the docs that you can not "trust" |
<script> | ||
import { get_event } from './data.remote.js'; | ||
const event = await get_event(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the line causing the lint failure:
packages/kit/test/apps/basics check: /home/runner/work/kit/kit/packages/kit/test/apps/basics/src/routes/remote/event/+page.svelte:4:16
packages/kit/test/apps/basics check: Error: 'await' expressions are only allowed within async functions and at the top levels of modules. (js)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhhhhh how do we fix that? does svelte-check
need to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I also get red squigglies in my editor, which I assume is because the VS Code extension is attempting to load config from the project root, rather than the nearest svelte.config.js
to the file in question. Is this a bug @dummdidumm?)
Ok, so it turns out enabling async rendering breaks a few tests, which we'll have to look into. Disabled it for now |
Yeah, just pushed something. I do almost wonder if we should just automatically re-run queries that depend on these values when they change, because I can see people doing that regardless |
We originally chose to omit
event.route
andevent.url
from remote functions, because we worried that it could create confusion — if a query's return value is based onevent.url
, it could become incorrect later when the URL changes and the query result doesn't.But it's too restrictive. It's often necessary to know which page a remote function was called from. For example if a query redirects to a
/login
page, it's nice to be able to include aredirectTo
parameter, and for that you need to know the page the user was trying to access:This PR makes that possible. Closes #14543.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits