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

adapter-node doesn't work correctly with paths.base #3726

Closed
vekunz opened this issue Feb 4, 2022 · 23 comments · Fixed by #4448
Closed

adapter-node doesn't work correctly with paths.base #3726

vekunz opened this issue Feb 4, 2022 · 23 comments · Fixed by #4448
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. paths.base bugs relating to `config.kit.paths.base`
Milestone

Comments

@vekunz
Copy link

vekunz commented Feb 4, 2022

Describe the bug

Basically, we have this svelte.config.js.

import adapter from '@sveltejs/adapter-node';

const config = {
    kit: {
        adapter: adapter(),
        paths: {
            base: '/basepath'
        },
    }
};
export default config;

When we now run svelte-kit build, it produces this folder structure (partly):

build
    client
        _app
            ...
    server
        ...
    static
        ...

the problem now is that the browser is not able to fetch the files in client and static. The node server returns status code 404 for these files.

The browser requests the files like this: http://localhost:3000/basepath/_app/....

If we manually modify the folder structure to this (manually add a basepath folder in client and static) it works fine.

build
    client
        basepath
            _app
                ...
    server
        ...
    static
        basepath
            ...

It seems that the adapter-node forgets to take paths.base into account for the static files.

By the way: If I start the build with svelte-kit preview, it works fine. Just starting it with node build does not work.

Reproduction

I created a simple demo app based on the npm init svelte@next command.

https://github.com/vekunz/svelte-paths-base-demo

Logs

basepath:1 GET http://localhost:3000/basepath 404 (Not Found)
basepath:9 GET http://localhost:3000/basepath/_app/assets/pages/__layout.svelte-ace81755.css net::ERR_ABORTED 404 (Not Found)
basepath:10 GET http://localhost:3000/basepath/_app/start-5dffe62e.js net::ERR_ABORTED 404 (Not Found)
basepath:19 GET http://localhost:3000/basepath/_app/assets/svelte-logo-87df40b8.svg 404 (Not Found)
basepath:13 GET http://localhost:3000/basepath/_app/error.svelte-bf7b1a86.js net::ERR_ABORTED 404 (Not Found)
basepath:11 GET http://localhost:3000/basepath/_app/chunks/vendor-ee294e9e.js net::ERR_ABORTED 404 (Not Found)
basepath:12 GET http://localhost:3000/basepath/_app/pages/__layout.svelte-1bb88ece.js net::ERR_ABORTED 404 (Not Found)

System Info

System:
    OS: Windows 10 10.0.18363
    CPU: (12) x64 Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
    Memory: 16.61 GB / 31.79 GB
  Binaries:
    Node: 14.18.1 - C:\Program Files\nodejs\node.EXE
    npm: 6.14.15 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 97.0.4692.99
    Edge: Spartan (44.18362.1593.0)
    Internet Explorer: 11.0.18362.1766
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.17
    @sveltejs/adapter-node: ^1.0.0-next.67 => 1.0.0-next.67
    @sveltejs/kit: next => 1.0.0-next.260
    svelte: ^3.46.0 => 3.46.4

Severity

blocking all usage of SvelteKit

Additional Information

It seems that in my example app moving the static files in a basepath subdirectory does not work also.

@joelhickok
Copy link

Same issue. Software is not usable for now.

@yannp00
Copy link

yannp00 commented Feb 17, 2022

Similar issues with adapter-static - see #3620 Incorrect build path for prerendered endpoints when setting a basepath

@bundabrg
Copy link

I actually question the reason for hard-coding the base path in a folder structure like this. If I run the app behind a reverse proxy I'd like to dynamically provide the prefix its listening to (through the x-forwaded-prefix header for example).

The workaround of fixing the build folder seems to solve the issue for the moment however

@bundabrg
Copy link

I've had a closer look at how the copy occurs. If you're simply wanting to copy to the correct folders then a small modification of the following to include the base could be done.

builder.log.minor('Copying assets');
builder.writeClient(`${out}/client`);
builder.writeServer(`${out}/server`);
builder.writeStatic(`${out}/static`);

This is probably the easiest way to solve both the static and node adapters though presumably the static adapter would place it in the root (as you would copy it into the correct base subfolder when deploying it, so the problem is actually the reverse of the node adapter) whereas the node adapter one needs to EITHER translate base on the fly (which means it could support an environment variable defining the base at runtime, which is supported by the override method exported from Server allowing you to override this at runtime, I think) OR it can just serve it from the hard coded compile time base folder. I don't really know which way is preferred so can't provide a PR and may need some discussion.

@joelhickok
Copy link

Instead of having to modify the source code as @bundabrg mentions, I am just using an npm deploy script to copy my node-adaptor build directory to a production directory, and move the problem directories to restructure at the same time. This has caused me to temporarily forget about this issue.

bundabrg added a commit to bundabrg/kit that referenced this issue Feb 24, 2022
* Add `base` to builder
* Prepend `builder.base` when copying Client, Static and Prerendered assets in adapter-node

Fixes sveltejs#3726
Related sveltejs#3620
@bundabrg
Copy link

I personally think that this is actually part of a bigger issue and simply copying to the correct folder whilst a quick fix is not a "good fix". I've added changes to my branch that does the quick fix but I don't like it.

When generating a static site we ultimately will be placing it either in the correct folder OR preferably behind a load-balancing reverse proxy that strips off the prefix and have it served from the root. So the prefix should not exist in the build folder at all but the code will need to know to add the prefix when referring to assets.

When generating a dynamic site it can be passed environment variables (or perhaps read a config file) where it gets its base prefix. In the same way the site could exist behind a reverse proxy that strips off the prefix OR it'll handle the prefix itself. It matters not as this is an implementation detail of the adapter. The code needs to know to add the prefix when referring to assets and presumably when doing SSR fetching it'll need to know to add/remove the prefix as necessary.

adapter-node uses Polka to serve its assets, executed with the following code:

const server = polka().use(
	// https://github.com/lukeed/polka/issues/173
	// @ts-ignore - nothing we can do about so just ignore it
	compression$1({ threshold: 0 }),
	handler
);

handler is provided as:

const handler = sequence(
	[
		serve(path.join(__dirname, '/client'), 31536000, true),
		serve(path.join(__dirname, '/static'), 0),
		serve(path.join(__dirname, '/prerendered'), 0),
		ssr
	].filter(Boolean)
);

The issue is that ssr will strip off base from the beginning of the URL but the three serve calls do not. The ssr function IMHO should not be doing this and instead everything should be mounted at the polka().use() call, something like this:

const server = polka().use(
        BASE_PATH,
	// https://github.com/lukeed/polka/issues/173
	// @ts-ignore - nothing we can do about so just ignore it
	compression$1({ threshold: 0 }),
	handler
);

where BASE_PATH would be set either by reading the config file or come in from an environment variable.

This way if I have a reverse proxy that strips prefixes I can have BASE_PATH set to '' but pass the actual base through a header like X-Forwarded-Prefix for when building URL's.

If I don't strip the prefix, then I can set BASE_PATH to the prefix so its handled inside Polka instead but otherwise the rest of the code remains the same.

Anyway, that's my 2c.

@Rich-Harris Rich-Harris added the bug Something isn't working label Apr 25, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone Apr 25, 2022
@Rich-Harris Rich-Harris added the paths.base bugs relating to `config.kit.paths.base` label Apr 25, 2022
@j6b72
Copy link

j6b72 commented Jun 12, 2022

This bug still exists in the current version. The workaround of moving both the client/_app folder and the contents of the client/static folder to the right place works though.

@shot-codes
Copy link

shot-codes commented Jun 23, 2022

As already mentioned, there is a related issue with the static adapter - #4528. The workaround of adjusting the directories didn't work for me but I found a workaround with the following.

<script>
  import { base } from '$app/paths';
</script>

<a href="{base}/">Home</a>
<a href="{base}/about">About</a>

@jcallaha
Copy link

@TheFoolishPupil - I tried your trick but I get links like /%7Bbase%7D/about - may be because I'm using a component to render nav and it is passed an array with href properties? If I switch to ${base}/about it works in dev but gets lost when I run preview or build (using static adapter)

@shot-codes
Copy link

@jcallaha, if I understand you correctly, you pass hrefs as a property to you nav component? In some sub-component where you actually render the tags you should be able to take whichever path was passed as a prop and prepend base to it. You could potentially do it before you pass the paths as properties, maybe somewhere in your javascript? perhaps I misunderstand your use case but their should be a way to get this work around to work for you. Maybe if you elaborate a bit more I could lend a hand.

@oetiker
Copy link

oetiker commented Jul 5, 2022

my workaround. it seems 'just moving the _app folder' will not work.

$ npx svelte-kit build
$ mkdir build/client/$base
$ ln -s ../_app build/client/$base
$ node build/index.js

@quasarchimaere
Copy link

afaik

my workaround. it seems 'just moving the _app folder' will not work.

$ npx svelte-kit build
$ mkdir build/client/$base
$ ln -s ../_app build/client/$base
$ node build/index.js

what i do is to set a temporary base-path in the vite config, and use a custom node script that replaces the occurences of the temporary base path in all the files of the build folder in addition to moving the _app (and static) folders into the directory structure that represents the basepath -> since i am using a docker container/kubernetes deployment scenario this works "fine" for my purposes

@seanlail
Copy link
Contributor

seanlail commented Jul 8, 2022

Not sure how everyone got it to work by moving client/_app. That doesn't work for me.

As @bundabrg suggested, I had to move quite a few folders.
I'm using Docker though, so it's part of the build step and seems to work well.
Brittle though - if those folders are renamed or something else is added it all breaks.

From To
/build/client/ /build/client/$base/
/build/prerendered/ /build/prerendered/$base/
/build/server/ /build/server/$base/
/build/static/ /build/static/$base/

Warning: I'm not sure the server folder should move though 🤔

@joelhickok
Copy link

joelhickok commented Jul 8, 2022

This is what I am doing to automate the directory structure updates. I chain this script onto my build command. This works for me and I am using this approach for all my Svelte Kit apps (for now).

@seanlail, I think I may still be importing base in my components too. I'll have to check if that is a necessary step or not. My script doesn't move server, and I'm pretty sure the server directory has nothing to do with this issue.

import { base } from '$app/paths'

<a href="{base}/about">About</a>
// snippet from svelte.config.js
kit: {
    adapter: adapter(),
    paths: {
        base: isDev ? '' : '/root-path',
    }
}
// snippet from package.json
"scripts": {
  "dev": "svelte-kit dev",
  "build": "svelte-kit build && node path_fix.js",
}
// path_fix.js
import fs from 'fs-extra'
import CONFIG from './svelte.config.js'

const appRoot = CONFIG.kit.paths.base // something like '/root-path'

const clientSrc = `./build/client/_app`
const clientDest = `./build/client${appRoot}/_app` // appRoot leads with a slash

if (fs.exists(clientDest)) {
    fs.removeSync(clientDest)
}

fs.moveSync(clientSrc, clientDest)

const staticSrc = `./build/static`
const staticDest = `./build/static${appRoot}`

if (fs.exists(staticDest)) {
    fs.removeSync(staticDest)
}

fs.mkdirsSync(staticDest)
const contents = fs.readdirSync(staticSrc)

contents
    .filter(entry => entry !== '.DS_Store')
    .forEach(entry => {
        fs.moveSync(`${staticSrc}/${entry}`, `${staticDest}/${entry}`)
    })

@olidacombe
Copy link

@joelhickok Thanks this works great! I had to swap two lines to avoid Error: Cannot move './build/static/mypath' to a subdirectory of itself, './build/static/mypath/mypath'.:

const contents = fs.readdirSync(staticSrc)
fs.mkdirsSync(staticDest)

@joelhickok
Copy link

@joelhickok Thanks this works great! I had to swap two lines to avoid Error: Cannot move './build/static/mypath' to a subdirectory of itself, './build/static/mypath/mypath'.:

const contents = fs.readdirSync(staticSrc)
fs.mkdirsSync(staticDest)

Whoops... yes, I had that problem too but corrected it. I thought I copied and pasted the corrected code, but maybe not. I'll check and update when I get a chance soon.

@fehnomenal
Copy link
Contributor

fehnomenal commented Jul 17, 2022

Oh, I really need this to be fixed. I cannot use the workaround mentioned earlier because my case is one level more complex as I need to use the generated middleware handler in another express server (directus). I cannot control exactly when to .use(handler) and they bind many other middlewares which means that I have to bind the sveltekit handler only to a single subpath (say /dashboard).

.use(handler) without setting paths.base

Both frontends (sveltekit and directus) are confused and nothing works...

.use(handler) with setting paths.base to /dashboard

Sveltekit frontend displays html pages but scripts are not found. But directus does not load correctly.

.use('/dashboard', handler) without setting paths.base

I can access the sveltekit frontend via /dashboard but internal links (href="{base}/about") are not working as base is empty and javascript cannot be loaded (urls do not contain /dashboard and thus are not handled by the middleware). Directus works without probems.

.use('/dashboard', handler) with setting paths.base to /dashboard

I can access kit via /dashboard/dashboard (yes duplicated) but the the about page is not available at /dashboard/about (which is build from href="{base}/about") but at /dashboard/dashboard/about. Basically I need to duplicate the base path. Strangely now the javascript files can be loaded correctly (at /dashboard/_app/immutable/start-e25cee58.js).

I found I can fix this by removing these lines

if (options.paths.base && !state.prerendering?.fallback) {
if (!decoded.startsWith(options.paths.base)) {
return new Response('Not found', { status: 404 });
}
decoded = decoded.slice(options.paths.base.length) || '/';
}

Now everything works as expected (i.e. /dashboard and /dashboard/about) at least with my minimal testing.

This is surely not the final solution as the dev server needs those lines and I guess the other adapters need them as well...


I got one step further by adding the base url to the request before passing it to the handler:

    app.use('/dashboard', (req, res, next) => {
      let requestProxy = new Proxy(req, {
        get(target, property) {
          if (property === 'url') {
            return target.baseUrl + (target.url === '/' ? '' : target.url);
          }

          return target[property];
        },
      });

      handler(requestProxy, res, next);
    });

Now the ssr function works correctly (because it strips the basepath we just added) but the static files are not correctly served (because sirv does not strip the basepath) as @bundabrg also noted in #3726 (comment)

@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Jul 19, 2022
@SylvainGarrigues
Copy link

Two comments:

  1. did anyone notice that when running npm run build with paths.base set to, say "/myapp" (sveltekit's documentation says it should not end with a slash), vite complains it should instead end with a slash? Something is wrong there (see line (!) "base" option should end with a slash.:
# vite build
vite v3.0.8 building for production...
✓ 52 modules transformed.
.svelte-kit/output/client/_app/immutable/assets/svelte-logo-87df40b8.svg                           1.85 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-cyrillic-ext-400-normal-3df7909e.woff2   15.40 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-cyrillic-400-normal-c7d433fd.woff2       8.89 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-greek-ext-400-normal-9e2fe623.woff2      7.33 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-greek-400-normal-a8be01ce.woff2          10.27 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-latin-400-normal-e43b3538.woff2          15.90 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-latin-ext-400-normal-6bfabd30.woff2      11.10 KiB
.svelte-kit/output/client/_app/immutable/assets/fira-mono-all-400-normal-1e3b098c.woff             75.55 KiB
.svelte-kit/output/client/vite-manifest.json                                                       7.60 KiB
.svelte-kit/output/client/_app/immutable/components/error.svelte-1aa22767.js                       1.54 KiB / gzip: 0.71 KiB
.svelte-kit/output/client/_app/immutable/components/pages/_layout.svelte-818ca664.js               4.50 KiB / gzip: 1.74 KiB
.svelte-kit/output/client/_app/immutable/modules/pages/_page.js-07d4ebd9.js                        0.07 KiB / gzip: 0.08 KiB
.svelte-kit/output/client/_app/immutable/modules/pages/about/_page.js-49d56463.js                  0.11 KiB / gzip: 0.11 KiB
.svelte-kit/output/client/_app/immutable/components/pages/_page.svelte-60cbe889.js                 5.46 KiB / gzip: 2.42 KiB
.svelte-kit/output/client/_app/immutable/components/pages/about/_page.svelte-4c2aaaa2.js           2.46 KiB / gzip: 1.11 KiB
.svelte-kit/output/client/_app/immutable/chunks/singletons-e5e57fbd.js                             0.05 KiB / gzip: 0.07 KiB
.svelte-kit/output/client/_app/immutable/components/pages/todos/_page.svelte-e9a45e5b.js           6.86 KiB / gzip: 2.88 KiB
.svelte-kit/output/client/_app/immutable/start-f6a2d763.js                                         22.91 KiB / gzip: 8.71 KiB
.svelte-kit/output/client/_app/immutable/chunks/index-67daab9a.js                                  0.43 KiB / gzip: 0.31 KiB
.svelte-kit/output/client/_app/immutable/chunks/stores-87dd2ac2.js                                 0.53 KiB / gzip: 0.31 KiB
.svelte-kit/output/client/_app/immutable/chunks/_page-d86dbbba.js                                  0.14 KiB / gzip: 0.14 KiB
.svelte-kit/output/client/_app/immutable/chunks/_page-20446a86.js                                  0.18 KiB / gzip: 0.16 KiB
.svelte-kit/output/client/_app/immutable/chunks/0-5939c936.js                                      0.15 KiB / gzip: 0.13 KiB
.svelte-kit/output/client/_app/immutable/chunks/1-3fa30a9b.js                                      0.14 KiB / gzip: 0.13 KiB
.svelte-kit/output/client/_app/immutable/chunks/index-98702734.js                                  11.48 KiB / gzip: 4.90 KiB
.svelte-kit/output/client/_app/immutable/chunks/2-230b0920.js                                      0.20 KiB / gzip: 0.15 KiB
.svelte-kit/output/client/_app/immutable/chunks/3-19d10a99.js                                      0.18 KiB / gzip: 0.15 KiB
.svelte-kit/output/client/_app/immutable/chunks/4-34bae440.js                                      0.18 KiB / gzip: 0.15 KiB
.svelte-kit/output/client/_app/immutable/assets/+page-5c3529b5.css                                 3.70 KiB / gzip: 1.03 KiB
.svelte-kit/output/client/_app/immutable/assets/+page-8bad58d3.css                                 1.45 KiB / gzip: 0.52 KiB
.svelte-kit/output/client/_app/immutable/assets/+page-9682aba9.css                                 0.11 KiB / gzip: 0.10 KiB
.svelte-kit/output/client/_app/immutable/assets/+layout-c589ff87.css                               4.84 KiB / gzip: 1.54 KiB
(!) "base" option should end with a slash.
vite v3.0.8 building SSR bundle for production...
✓ 58 modules transformed.
.svelte-kit/output/server/vite-manifest.json                    3.04 KiB
.svelte-kit/output/server/index.js                              57.72 KiB
.svelte-kit/output/server/entries/pages/_layout.svelte.js       4.09 KiB
.svelte-kit/output/server/entries/fallbacks/error.svelte.js     0.60 KiB
.svelte-kit/output/server/entries/pages/_page.svelte.js         7.10 KiB
.svelte-kit/output/server/entries/pages/_page.js                0.05 KiB
.svelte-kit/output/server/entries/pages/about/_page.svelte.js   1.25 KiB
.svelte-kit/output/server/entries/pages/about/_page.js          0.15 KiB
.svelte-kit/output/server/entries/pages/todos/_page.svelte.js   5.92 KiB
.svelte-kit/output/server/entries/pages/todos/_page.server.js   1.22 KiB
.svelte-kit/output/server/chunks/index.js                       4.15 KiB
.svelte-kit/output/server/chunks/index2.js                      0.73 KiB
.svelte-kit/output/server/chunks/index3.js                      1.29 KiB
.svelte-kit/output/server/chunks/stores.js                      0.81 KiB
.svelte-kit/output/server/chunks/hooks.js                       0.49 KiB
...
  1. If the fix for this bug is to move some _app directories in some newly created directory (named like paths.base), why isn't the fix obvious (in adapter-node or in the build script)?

@DrDanRyan
Copy link

I think I figured out where the change needs to happen. In adapter-node/index.js the line that reads:

builder.writeClient(`${out}/client`);

should change to

builder.writeClient(`${out}/client${builder.config.kit.paths.base}`);

I suspect the writePrerendered method call might need modifying too, but I don't have an example app using prerendering to test that on.

I also suspect their is a similar change in adapter-static that will fix the same issue.

@gerhardcit
Copy link

gerhardcit commented Aug 23, 2022

I think I figured out where the change needs to happen. In adapter-node/index.js the line that reads:

builder.writeClient(`${out}/client`);

should change to

builder.writeClient(`${out}/client${builder.config.kit.paths.base}`);

I suspect the writePrerendered method call might need modifying too, but I don't have an example app using prerendering to test that on.

I also suspect their is a similar change in adapter-static that will fix the same issue.

This works, but you still need to specifically set
export const prerender = false;
for
npm run build to work with out errors.

otherwise you stil get this

.svelte-kit/output/server/chunks/hooks.js                          0.49 KiB
file:///tests/sveltekit-container/node_modules/@sveltejs/kit/src/core/prerender/prerender.js:48
                                throw new Error(format_error(details, config));
                                      ^

Error: 404 / does not begin with `base`, which is configured in `paths.base` and can be imported from `$app/paths` (linked from /dash)

with sveltekig.config.js set like this

import adapter from '@sveltejs/adapter-node';
import preprocess from 'svelte-preprocess';

/** @type {import('@sveltejs/kit').Config} */
const config = {
	// Consult https://github.com/sveltejs/svelte-preprocess
	// for more information about preprocessors
	preprocess: preprocess(),

	kit: {
		adapter: adapter(),

		paths: {
			base: "/dash"
		},
		prerender: {
			default: false
		},
		// Override http methods in the Todo forms
		methodOverride: {
			allowed: ['PATCH', 'DELETE']
		}
	}
};

export default config;

Makes me wonder.
Why do most build tools assume apps will run on the root? "Hello World" apps maybe, but not "Real World" apps.

base should ALWAYS be a variable, defaulting to "/" maybe yes, but never assume it's not there.

@charbelnicolas
Copy link

20220831_072212

I get the same warning: "(!) "base" option should end with a slash."

@koryphaee
Copy link

Any updates here? I am currently evaluating using Svelte(Kit) in our new app and this would be a showstopper.

dummdidumm pushed a commit that referenced this issue Oct 12, 2022
fixes #4442, fixes #2843, fixes #3726

Write static files to a folder that takes into account the base path value
@dummdidumm
Copy link
Member

The bug the majority in this issue have is fixed now, the base path is taken into account when writing the files to disk. This doesn't suffice for people with reserved proxies which strip the base path or something related. #7242 tracks that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. paths.base bugs relating to `config.kit.paths.base`
Projects
None yet
Development

Successfully merging a pull request may close this issue.