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

Page reloads if an endpoint exist on the same URL even without GET method #2012

Closed
JulienPradet opened this issue Jul 25, 2021 · 1 comment
Closed
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.

Comments

@JulienPradet
Copy link

JulienPradet commented Jul 25, 2021

Describe the bug

Might be related to #1711.

See Reproduction for a TLDR of the bug itself.

I am currently working on creating a form that can be both sent via ajax or via a classic form submission when JavaScript is not active. This is just like the usecase mentioned in #1711.

I'm implementing a login flow that for technical reasons needs to be an old-fashioned form submit. If the username and password that are entered fail to authenticate, I'd like the response to the POST to be the same login page, but with the username already pre-filled with what the user entered last time, and with a specific error message about what happened.

So my POST endpoint fallthrough a svelte page and pass information using getSession, waiting for a more idomatic solution in #1711. It works, however, there is currently a bug when I try to navigate to my form page.

Since there is an endpoint and a page matching the same URL, the Renderer, when it updates will consider that the first route matching the URL is an endpoint and thus will reload. However, my endpoint only have a POST method and no GET method. So it should never conflict with the client side navigation.

My suggestion would be a change during the generation of the manifest.js. If we include the type of methods handled by each route we would be able to filter out the endpoints that don't have any GET method. This new information could then be used in Router.parse. Not sure how you'd like to do such a change. Parsing the AST of the endpoint file to see what are the named exports? If so I could try to tackle a PR around this change if it can help. Would be kind of an impactful change so not sure if that's something you'd like.

parse(url) {
if (this.owns(url)) {
const path = decodeURIComponent(url.pathname.slice(this.base.length) || '/');
const routes = this.routes.filter(([pattern]) => pattern.test(path));

Reproduction

Here is a repository demonstrating the issue: https://github.com/JulienPradet/sveltekit-page-reload-bug

Current behavior:

  • The page reloads after clicking on the link

Expected behavior:

  • The page should indeed go to the login page but it shouldn't reload after the navigation.

The form available in the repository is not relevant to the issue itself but I added it to explain why I need to have the /login endpoint that have the same URL as my page.

Logs

Not relevant

System Info

System:
    OS: Linux 4.14 Manjaro Linux
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
    Memory: 524.41 MB / 15.41 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.5.0 - ~/.nvm/versions/node/v16.5.0/bin/node
    npm: 7.19.1 - ~/.nvm/versions/node/v16.5.0/bin/npm
  Browsers:
    Firefox: 87.0
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.134 
    svelte: ^3.34.0 => 3.40.1

Severity

serious, but I can work around it

Additional Information

About the severity, I can disable my no-javascript form and only use ajax, but I couldn't find a workaround to make it work with the no-javascript form. So kinda blocking even though it's still a rare usecase in the javascript community (sadly). But since #1711 exists, I guess it's still something you'd like to consider in Svelte Kit.

@Conduitry Conduitry added the feature / enhancement New feature or request label Jul 26, 2021
@benmccann benmccann added the p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. label Aug 4, 2021
@mrkishi
Copy link
Member

mrkishi commented Apr 5, 2022

This use-case is currently supported by page endpoints.

The fallthrough workaround doesn't even work anymore since that feature was removed.

@mrkishi mrkishi closed this as completed Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

No branches or pull requests

4 participants