Skip to content

Conversation

@dummdidumm
Copy link
Member

fixes #8301

Not sure if this is the best approach, therefore no test yet

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. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@changeset-bot
Copy link

changeset-bot bot commented Jan 12, 2023

🦋 Changeset detected

Latest commit: 037309f

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
Copy link
Member

If we were to do this I think we'd want a failed_preloads.clear() somewhere, most likely after any successful navigation. I do somewhat lean towards arguing that this is the developer's responsibility to address with data-sveltekit-preload-data="off", though I could probably be persuaded in the other direction.

@dummdidumm
Copy link
Member Author

I didn't add a clear because if you have a layout with a link, that link would try to be prefetched again (and fail again) after each navigation. Under what circumstances could the approach of only deleting the id in question lead to something not getting prefetched again? (possible answer: with a redirect; that probably needs a check to delete this as well)

If we don't want to change this, then we should at least print a more helpful message to the console at dev time. Something like "if this is a link that is very likely to fail while prefetching, consider setting data-sveltekit-preload-data="off" on it".

@Rich-Harris
Copy link
Member

Under what circumstances could the approach of only deleting the id in question lead to something not getting prefetched again?

Say I fail to navigate to /admin, then navigate to /login, then attempt to navigate to /admin again. Following the successful navigation to /login I'd expect subsequent links to be preloaded

@dummdidumm
Copy link
Member Author

Yeah that makes sense.

Since this is all highly app-specific, and hard to find a one-size-fits-all-solution (your case for example is more conservative, but the related issue would probably prefer my solution), I think a dev time warning is the better solution.

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.

Prefetch repeatedly refetching after a 40x from a route

4 participants