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

fix: outdated endpoint in todo example app #1344

Merged
merged 9 commits into from
May 5, 2021
Merged

fix: outdated endpoint in todo example app #1344

merged 9 commits into from
May 5, 2021

Conversation

ignatiusmb
Copy link
Member

Fixes #1341

Also add global interface to be passed in RequestHandler and fix lint errors.

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

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

@Rich-Harris
Copy link
Member

Thank you. I'm in two minds about the global Locals interface — people will certainly need to change it, but right now it's a little undiscoverable. I wander if we should instead export it from src/lib/types.d.ts and do

import type { Locals } from '$lib/types';

or something instead?

@Rich-Harris
Copy link
Member

(The flip side of course is that you might well prefer to make it global in your app. Tough one.)

@ignatiusmb
Copy link
Member Author

I was thinking the same, some of the considerations for making it global was (as mentioned) convenience and for an example on the possibility of adding global types/interfaces (since import type { } from 'module' is already demonstrated by RequestHandler).

Then again, having it exported from src/lib/types.d.ts is definitely more explicit and discoverable.

@Rich-Harris
Copy link
Member

Perhaps the best-of-both-worlds solution is to use $lib/types but have a comment in there above the interface describing how to make it globally available if that's what the developer prefers?

@ignatiusmb
Copy link
Member Author

I love it, that's genius. Done!

@Rich-Harris
Copy link
Member

Perfect, thank you! One last thing I forgot to mention before — could you run pnpx changeset so that we can release a new version of create-svelte? (Would do it myself but I've had bad results editing PRs by working on branches of forks)

@Conduitry
Copy link
Member

@Rich-Harris I've added a changeset.

@Rich-Harris Rich-Harris merged commit 5ed3ed2 into sveltejs:master May 5, 2021
@ignatiusmb ignatiusmb deleted the gh-1341 branch May 6, 2021 00:58
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.

userid no longer available in request.context
3 participants