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

Can't fetch assets using Kits fetch and relative path #12081

Closed
mastermakrela opened this issue Apr 4, 2024 · 3 comments · Fixed by #12113
Closed

Can't fetch assets using Kits fetch and relative path #12081

mastermakrela opened this issue Apr 4, 2024 · 3 comments · Fixed by #12113
Labels
bug Something isn't working

Comments

@mastermakrela
Copy link
Contributor

Describe the bug

According to the documentation (here and here), kits fetch should be able to handle relative paths on the server.

This works in development, but not in production (I've tested Cloudflare and node adapters).

If I do this:

// +server.ts
import asset from "$lib/image.png";

export const GET = async ({ fetch: sk_fetch }) => {
	const _asset = await sk_fetch(asset);

	// we always end up here
	if (!_asset.ok)  return error(404, "Not found");
	
	return _asset;
};

The response is always this exact 404 (as far as I can tell):

if (decoded.startsWith(`/${options.app_dir}`)) {
return text('Not found', { status: 404 });
}

We end up here, because the server_fetch doesn't recognise this path as an asset.
Following check seams to be the problem:

const is_asset = manifest.assets.has(filename);
const is_asset_html = manifest.assets.has(filename_html);
if (is_asset || is_asset_html) {

as it doesn't check manifest._.server_assets


I've created an exhaustive reproduction that shows differences between sk/global fetch and absolute/relative path here:
https://github.com/mastermakrela/sveltekit-relative-assets-fetch-bug


Solutions considered

  1. Use read

    • works only for node
    • currently can't be implemented for Cloudflare, because read is sync and Cloudflares env.ASSETS.fetch isn't
  2. Change is_asset check in Kit to

    const is_asset =
    	manifest.assets.has(filename) ||
    	Object.prototype.hasOwnProperty.call(manifest._.server_assets, filename);
  3. Change is_static_asset check in Cloudflare adapter to:

    is_static_asset =
    	manifest.assets.has(filename) ||
    	manifest.assets.has(filename + '/index.html') ||
    	Object.prototype.hasOwnProperty.call(manifest._.server_assets, filename);
  4. Use global fetch


Semantically async read + Cloudflare implementation, would be most satisfying, but it's probably leas viable, because of breaking change.

But independent of read, not being able to fetch assets using relative path on server seams to be a bug, that should be fixed (or if not at least a warning in dev mode would be nice).

Reproduction

https://github.com/mastermakrela/sveltekit-relative-assets-fetch-bug

Logs

No response

System Info

System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1
    Memory: 69.52 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.7.1 - /opt/homebrew/bin/node
    npm: 10.5.0 - /opt/homebrew/bin/npm
    pnpm: 8.15.6 - /opt/homebrew/bin/pnpm
    bun: 1.0.25 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 123.1.64.109
    Chrome: 123.0.6312.87
    Edge: 112.0.1722.54
    Safari: 17.4.1
  npmPackages:
    @sveltejs/adapter-auto: ^3.2.0 => 3.2.0 
    @sveltejs/adapter-cloudflare: ^4.2.0 => 4.2.0 
    @sveltejs/adapter-node: ^5.0.1 => 5.0.1 
    @sveltejs/vite-plugin-svelte: ^3.0.2 => 3.0.2 
    svelte: 5.0.0-next.90 => 5.0.0-next.90 
    vite: ^5.2.7 => 5.2.7

Severity

serious, but I can work around it

Additional Information

No response

@Rishab49
Copy link

Rishab49 commented Apr 5, 2024

I Think sk_fetch is causing the problem
you should do this instead

export const GET: RequestHandler = async ({ fetch , url, platform }) => {

It works for me

@mastermakrela
Copy link
Contributor Author

@Rishab49 sk_fetch is just a rename, the code above is equivalent to

export const GET = async (event) => {
    const sk_fetch = event.fetch;
    ...
}

I renamed it to clearly differentiate global fetch and the one from SvelteKit here


It works for me

Did you test it with Cloudflare adapter & wrangler? If yes, could you share a minimal setup? That would really help me.

@Rishab49
Copy link

Rishab49 commented Apr 5, 2024

oh I forgot to test it with wrangler(apologies), its a bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants