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

Sveltekit throws developer warning when using correct fetch from load-function #5031

Closed
MathiasWP opened this issue May 23, 2022 · 4 comments · Fixed by #5041
Closed

Sveltekit throws developer warning when using correct fetch from load-function #5031

MathiasWP opened this issue May 23, 2022 · 4 comments · Fixed by #5041
Milestone

Comments

@MathiasWP
Copy link
Contributor

Describe the bug

In #4958 a helper warning was introduced if the window.fetch method is used instead of the fetch method provided by the load function. However, this warning is logged even though the correct fetch method is used.

Reproduction

https://github.com/MathiasWP/sveltekit-window-fetch-warning-bug

Logs

No response

System Info

System:
    OS: macOS 11.4
    CPU: (8) x64 Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz
    Memory: 21.08 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 8.1.0 - /usr/local/bin/npm
  Browsers:
    Brave Browser: 101.1.38.111
    Chrome: 101.0.4951.64
    Edge: 101.0.1210.53
    Firefox: 100.0
    Safari: 14.1.1
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.44 
    @sveltejs/kit: next => 1.0.0-next.338 
    svelte: ^3.44.0 => 3.48.0

Severity

annoyance

Additional Information

No response

@elliott-with-the-longest-name-on-github
Copy link
Contributor

Not sure why this is happening, but I can confirm that commit d7cb8b8 (warn if load uses window.fetch) is where this was introduced, i.e. this is not a regression caused by a later commit.

@mrkishi
Copy link
Member

mrkishi commented May 23, 2022

Note: this is about initial_fetch using the patched window.fetch instead of the original.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

@mrkishi

You're totally correct -- I just nailed it down. The easy fix is this: https://github.com/tcc-sejohnson/kit/tree/sejohnson-fix-fetch-warning-on-initial-fetch

The slightly-harder fix is to pass fetch to initial_fetch (adjust the initial_fetch signature to accept fetch).

Given that this is a dev-mode thing for ergonomics, I don't see much of an issue just checking the stack trace for initial_fetch as seen in the linked branch. If we want to do that, I'll open a PR. If not, happy to make the adjustments necessary to not use the patched window.fetch.

I'm not entirely sure how to test the changes, though -- this would only happen before the app is started...

@Rich-Harris
Copy link
Member

I have a fix in a branch, one sec...

Rich-Harris added a commit that referenced this issue May 23, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone May 23, 2022
Rich-Harris added a commit that referenced this issue May 23, 2022
* failing test for #5031

* move fetch logic into separate module - fixes #5031

* changeset

* remove .only
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 a pull request may close this issue.

4 participants