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

Server asset path mismatch when running vite preview #11020

Closed
eltigerchino opened this issue Nov 12, 2023 · 14 comments · Fixed by #11649
Closed

Server asset path mismatch when running vite preview #11020

eltigerchino opened this issue Nov 12, 2023 · 14 comments · Fixed by #11649
Labels
breaking change needs-decision Not sure if we want to do this yet, also design work needed pkg:adapter-node vite

Comments

@eltigerchino
Copy link
Member

eltigerchino commented Nov 12, 2023

Describe the bug

Typically, we're suppose to access a server asset by joining process.cwd() and the path of the asset imported and handled by Vite.

// +page.server.js
import path from 'node:path';
import fs from 'node:fs';
import text_file from './file.txt';

export async function load() {
    // we can get the absolute path with `process.cwd()`
	const filepath = path.join(process.cwd(), text_file);
	const text = fs.readFileSync(filepath, { encoding: 'utf-8' });
	return { text };
}

However, this doesn't work when running vite preview or node build with the Node adapter.

// +page.server.js
import path from 'node:path';
import fs from 'node:fs';
import text_file from './file.txt';

export async function load() {
    // the filepath is incorrect because the path returned by `process.cwd()` is not near the server assets.
    // So we need to change the path when not in dev??
	const filepath = path.join(process.cwd(), text_file);
	const text = fs.readFileSync(filepath, { encoding: 'utf-8' });
	return { text };
}

This can be fixed by changing the current directory when vite preview is run. Hence, the filepath would correctly return /.svelte-kit/output/server/_app/... instead of the project root /_app/... which doesn't exist.

const dir = join(svelte_config.kit.outDir, 'output/server');

if (!fs.existsSync(dir)) {
	throw new Error(`Server files not found at ${dir}, did you run \`build\` first?`);
}

+ // always set the working directory to where our server assets are located
+ process.chdir(dir)

https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/vite/preview/index.js

A similar thing occurs when we use adapter-node. The path returned by process.cwd() keeps changing depending on from which directory we start the server. This could be considered the intended behaviour, but it also leads to unwanted behaviour (such as being unable to correctly resolve the filepath as shown in the example above).

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-y1duvx?file=src%2Froutes%2F%2Bpage.server.js

  1. Run the application with pnpm dev, this has no issue.
  2. Build the application with pnpm build.
  3. Preview the build with pnpm preview. The page cannot render because the filepath is incorrect in the load function.

Logs

Error: ENOENT: no such file or directory, open '/home/projects/sveltejs-kit-template-default-y1duvx/_app/immutable/assets/file.c0535e4b.txt'
    at Object.openSync (https://sveltejskittemplatedefaulty1du-uwot.w-credentialless.staticblitz.com/blitz.5a198b5c.js:49:16217)
    at Object.readFileSync (https://sveltejskittemplatedefaulty1du-uwot.w-credentialless.staticblitz.com/blitz.5a198b5c.js:49:17820)
    at load (file:///home/projects/sveltejs-kit-template-default-y1duvx/.svelte-kit/output/server/entries/pages/_page.server.js:21:32)
    at load_server_data (file:///home/projects/sveltejs-kit-template-default-y1duvx/.svelte-kit/output/server/index.js:1119:42)
    at eval (file:///home/projects/sveltejs-kit-template-default-y1duvx/.svelte-kit/output/server/index.js:2537:24) {
  syscall: 'open',
  errno: -2,
  code: 'ENOENT',
  path: '/home/projects/sveltejs-kit-template-default-y1duvx/_app/immutable/assets/file.c0535e4b.txt'
}
Error: ENOENT: no such file or directory, open '/home/projects/sveltejs-kit-template-default-y1duvx/_app/immutable/assets/file.c0535e4b.txt'
    at Object.openSync (https://sveltejskittemplatedefaulty1du-uwot.w-credentialless.staticblitz.com/blitz.5a198b5c.js:49:16217)
    at Object.readFileSync (https://sveltejskittemplatedefaulty1du-uwot.w-credentialless.staticblitz.com/blitz.5a198b5c.js:49:17820)
    at load (file:///home/projects/sveltejs-kit-template-default-y1duvx/.svelte-kit/output/server/entries/pages/_page.server.js:21:32)
    at load_server_data (file:///home/projects/sveltejs-kit-template-default-y1duvx/.svelte-kit/output/server/index.js:1119:42)
    at eval (file:///home/projects/sveltejs-kit-template-default-y1duvx/.svelte-kit/output/server/index.js:2537:24) {
  syscall: 'open',
  errno: -2,
  code: 'ENOENT',
  path: '/home/projects/sveltejs-kit-template-default-y1duvx/_app/immutable/assets/file.c0535e4b.txt'
}

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.9.2 - /usr/local/bin/pnpm
  npmPackages:
    @sveltejs/adapter-node: ^1.3.1 => 1.3.1 
    @sveltejs/kit: ^1.20.4 => 1.27.5 
    svelte: ^4.0.5 => 4.2.3 
    vite: ^4.4.2 => 4.5.0

Severity

serious, but I can work around it

@eltigerchino eltigerchino added bug Something isn't working pkg:adapter-node labels Nov 12, 2023
@Conduitry
Copy link
Member

Hm. This feels to me like something that shouldn't be our responsibility.

IIRC, running npm run <whatever> will set the CWD to the directory containing the relevant package.json file before running the script. Does that match what you're seeing?

For the production output from the Node adapter, I don't feel like the generated code should default to changing the CWD at all. It should just accept what the user called the built code with. If you have particular CWD needs, you can put something at the top-level in your hooks file.

In any case, changes to these defaults (whether in dev/preview mode, or in Node adapter output) would be a breaking change for anyone who is already resolving anything relative to the CWD.

@eltigerchino
Copy link
Member Author

eltigerchino commented Nov 12, 2023

IIRC, running npm run <whatever> will set the CWD to the directory containing the relevant package.json file before running the script. Does that match what you're seeing?

Yes, npm run preview consistently runs from the root of the project.

In any case, changes to these defaults (whether in dev/preview mode, or in Node adapter output) would be a breaking change for anyone who is already resolving anything relative to the CWD.

That's true, but I hope we'd at least change the default for preview mode to remove one of these steps.

// +page.server.js
import path from 'node:path';
import fs from 'node:fs';
import text_file from './file.txt';
import { env } from '$env/dynamic/private';

let filepath;

if (!dev && env.NODE) {
  // `node build`
  filepath = path.join(process.cwd(), 'build/server', text_file);
}
else if (!dev) {
  // `npm run preview`
  filepath = path.join(process.cwd(), '.svelte-kit/output/server', text_file);
}
else {
  // `npm run dev`
  filepath = path.join(process.cwd(), text_file);
}

export async function load() {
	const text = fs.readFileSync(filepath, { encoding: 'utf-8' });
	return { text };
}

@eltigerchino eltigerchino added breaking change and removed bug Something isn't working labels Nov 12, 2023
@eltigerchino eltigerchino added vite needs-decision Not sure if we want to do this yet, also design work needed labels Dec 1, 2023
@eltigerchino eltigerchino added this to the 2.0 milestone Dec 3, 2023
@dummdidumm
Copy link
Member

If at all we should only change this for preview - I'm with Conduitry on this one. The question is whether it would hide a bug that then only appears in production. In other words, how likely is it that people would start their node server from the wrong cwd so that this breaks?

@eltigerchino
Copy link
Member Author

eltigerchino commented Dec 8, 2023

Sounds good. Should we at least move the server assets (or all server files) to the same directory as the recommended cwd the node process runs at? Currently, the assets live in a subfolder named “server/…”, so a prod build with adapter-node would need to prepend every asset path with “server/“

@dummdidumm
Copy link
Member

So that means that fs.readFileSync doesn't work at all without adjustments currently for adapter-node? I'm a bit surprised this hasn't been reported before then.

@eltigerchino
Copy link
Member Author

eltigerchino commented Dec 8, 2023

So that means that fs.readFileSync doesn't work at all without adjustments currently for adapter-node? I'm a bit surprised this hasn't been reported before then.

That’s right. I only came across this when someone asked for a demo of a file upload to a node server with sveltekit. I wonder if people have just been working around by changing the path, starting the node process in the server folder, or moving the files with a postbuild script.

@dummdidumm
Copy link
Member

Rich had the nice idea of making this a function cwd you can import from @sveltejs/kit/node which basically does the if-else-dance outlined in #11020 (comment) . We would optionally allow to pass in the .svelte-kit/ thingy because maybe for whatever reason you changed that config, but other than that.

@Conduitry
Copy link
Member

Would the function return a consistent root directory, or would it actually chdir to that directory? Either way, I think cwd isn't an ideal name, because it's not really returning the current working directory.

@eltigerchino
Copy link
Member Author

eltigerchino commented Dec 8, 2023

I'm not sure I fully understand. Would that solve both the adapter-node specific case and the adapter agnostic vite preview case?

@dummdidumm
Copy link
Member

It would need to be written in such a way that it would. I guess it actually would be good to not touch vite preview in this case because it surfaces the error earlier ("oh this isn't the path I thought it would be, what to do? Oh, SvelteKit has something for me here, nice")

@Rich-Harris
Copy link
Member

I misunderstood the problem when I proposed this earlier. I thought the issue was that process.cwd() was the thing that was different between dev, preview and prod and we were trying to enable this...

path.join(cwd(), 'file-in-my-project-directory.text');

...but that's not the case – process.cwd() is the same, but text_file is different because it's an asset URL. Sounds like what we actually need here is something like this:

// +page.server.js
import path from 'node:path';
import fs from 'node:fs';
+import { serverAssets } from '$app/paths';
import text_file from './file.txt';

export async function load() {
    // we can get the absolute path with `process.cwd()`
-	const filepath = path.join(process.cwd(), text_file);
+	const filepath = path.join(serverAssets, text_file);
	const text = fs.readFileSync(filepath, { encoding: 'utf-8' });
	return { text };
}

Seems simple enough to implement. The challenge is making it visible — how do we force people to use serverAssets when it works as expected in dev? We need to make it fail early. Browser-side we make this work by serving assets inside /_svelte_kit_assets so you have to do fetch(${assets}/data.json) (if assets is specified); is there some clever trick we can apply on the server in dev?

@Conduitry
Copy link
Member

The only options I can think of right now are to purposefully process.chdir() to somewhere weird where nothing will automatically resolve (which seems very rude), or to monkeypatch fs functions (which seems very error-prone).

@Rich-Harris
Copy link
Member

I was thinking more along the lines of 'convince Vite to stick a prefix in front of imported asset URLs', so that text_file in the example above would be /_sveltekit_server_assets/text_file.txt (and then we do something fucked up with symlinks to make that work?). no idea if such a thing is possible

@Rich-Harris
Copy link
Member

I'm inclined to remove this from the 2.0 milestone, since I don't see how we can fix it without adding something like serverAssets, which would be a non-breaking change. Any objections?

@Rich-Harris Rich-Harris removed this from the 2.0 milestone Dec 11, 2023
@eltigerchino eltigerchino changed the title Inconsistent current working directory when running built application Asset path mismatch when running vite preview Jan 16, 2024
@eltigerchino eltigerchino changed the title Asset path mismatch when running vite preview Server asset path mismatch when running vite preview Jan 16, 2024
@eltigerchino eltigerchino linked a pull request Jan 17, 2024 that will close this issue
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change needs-decision Not sure if we want to do this yet, also design work needed pkg:adapter-node vite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants