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

feat: Move hooks.{js|ts} init to Server.init instead of Server.respond #6179

Merged
merged 22 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fa53fbb
feat: Move hooks.js initialization into Server.init
elliott-with-the-longest-name-on-github Aug 23, 2022
02c5764
fix: Server type
elliott-with-the-longest-name-on-github Aug 23, 2022
3bfd40c
feat: Await Server.init in adapters
elliott-with-the-longest-name-on-github Aug 23, 2022
286648a
feat: Await server.init in preview
elliott-with-the-longest-name-on-github Aug 23, 2022
204acb1
fix: Init server during prerender
elliott-with-the-longest-name-on-github Aug 23, 2022
16cdc7b
feat: Init hooks when dev mode starts instead of on first request
elliott-with-the-longest-name-on-github Aug 23, 2022
f8bd413
fix: Prerendering, but better
elliott-with-the-longest-name-on-github Aug 23, 2022
ea9c218
feat: Test env vars in prerender
elliott-with-the-longest-name-on-github Aug 23, 2022
c6a7f6d
fix: Remove console.log
elliott-with-the-longest-name-on-github Aug 23, 2022
7f31360
fix: Import in hooks.js of basics test app
elliott-with-the-longest-name-on-github Aug 23, 2022
3fba16e
changeset
elliott-with-the-longest-name-on-github Aug 23, 2022
2c7e2a8
Update packages/adapter-netlify/src/serverless.js
elliott-with-the-longest-name-on-github Aug 23, 2022
9583fbc
Update packages/kit/src/vite/build/build_server.js
elliott-with-the-longest-name-on-github Aug 23, 2022
6cb779c
feat: Enable top-level await in `adapter-node`
elliott-with-the-longest-name-on-github Aug 23, 2022
8ef2f83
feat: Add warning comment for future devs
elliott-with-the-longest-name-on-github Aug 23, 2022
7afab3e
fix: Init env prior to hooks
elliott-with-the-longest-name-on-github Aug 23, 2022
6131dd3
Merge branch '6147/sejohnson-move-hooks-init' of github.com:sveltejs/…
elliott-with-the-longest-name-on-github Aug 23, 2022
a8c62b2
fix: Remove dear Rich
elliott-with-the-longest-name-on-github Aug 23, 2022
23a9206
Merge branch 'master' into 6147/sejohnson-move-hooks-init
elliott-with-the-longest-name-on-github Aug 23, 2022
580ffd2
fix: Apparently didn't like my tsconfig
elliott-with-the-longest-name-on-github Aug 23, 2022
e9286ad
fix: Move env and hooks initialization later
elliott-with-the-longest-name-on-github Aug 23, 2022
5b9b0db
move user_hooks init back from whence it came
Rich-Harris Aug 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/curvy-students-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@sveltejs/adapter-cloudflare': patch
'@sveltejs/adapter-cloudflare-workers': patch
'@sveltejs/adapter-netlify': patch
'@sveltejs/adapter-node': patch
'@sveltejs/adapter-vercel': patch
'@sveltejs/kit': patch
---

[feat] Moved hooks.js initialization from Server.respond into Server.init
2 changes: 1 addition & 1 deletion packages/adapter-cloudflare-workers/files/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default {
* @param {any} context
*/
async fetch(req, env, context) {
server.init({ env });
await server.init({ env });

const url = new URL(req.url);

Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-cloudflare/src/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const prefix = `/${manifest.appDir}/`;
/** @type {import('worktop/cfw').Module.Worker<{ ASSETS: import('worktop/cfw.durable').Durable.Object }>} */
const worker = {
async fetch(req, env, context) {
server.init({ env });
await server.init({ env });
// skip cache if "cache-control: no-cache" in request
let pragma = req.headers.get('cache-control') || '';
let res = !pragma.includes('no-cache') && (await Cache.lookup(req));
Expand Down
7 changes: 6 additions & 1 deletion packages/adapter-netlify/src/serverless.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@ import { split_headers } from './headers';
export function init(manifest) {
const server = new Server(manifest);

server.init({
let init_promise = server.init({
env: process.env
});

return async (event, context) => {
if (init_promise !== null) {
await init_promise;
init_promise = null;
}

const response = await server.respond(to_request(event), {
platform: { context },
getClientAddress() {
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-node/src/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { env } from './env.js';
/* global ENV_PREFIX */

const server = new Server(manifest);
server.init({ env: process.env });
await server.init({ env: process.env });
const origin = env('ORIGIN', undefined);
const xff_depth = parseInt(env('XFF_DEPTH', '1'));

Expand Down
3 changes: 2 additions & 1 deletion packages/adapter-node/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"noImplicitAny": true,
"allowSyntheticDefaultImports": true,
"moduleResolution": "node",
"module": "es2020",
"module": "es2022",
"target": "es2017",
"baseUrl": ".",
"paths": {
"@sveltejs/kit": ["../kit/types/index"]
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-vercel/files/serverless.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ installPolyfills();

const server = new Server(manifest);

server.init({
await server.init({
env: process.env
});

Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/core/prerender/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { load_config } from '../config/index.js';
* @typedef {import('types').Logger} Logger
*/

const [, , client_out_dir, results_path, manifest_path, verbose] = process.argv;
const [, , client_out_dir, results_path, manifest_path, verbose, env] = process.argv;

prerender();

Expand Down Expand Up @@ -159,6 +159,7 @@ export async function prerender() {
});

const server = new Server(manifest);
await server.init({ env: JSON.parse(env) });

const error = normalise_error_handler(log, config);

Expand Down
19 changes: 12 additions & 7 deletions packages/kit/src/vite/build/build_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ export class Server {
};
}

init({ env }) {
/**
* Take care: Some adapters may have to call \`Server.init\` per-request to set env vars,
* so anything that shouldn't be rerun should be wrapped in an \`if\` block to make sure it hasn't
* been done already.
*/
async init({ env }) {
const entries = Object.entries(env);

const prv = Object.fromEntries(entries.filter(([k]) => !k.startsWith('${
Expand All @@ -99,12 +104,6 @@ export class Server {
set_public_env(pub);

this.options.public_env = pub;
}

async respond(request, options = {}) {
if (!(request instanceof Request)) {
throw new Error('The first argument to server.respond must be a Request object. See https://github.com/sveltejs/kit/pull/3384 for details');
}

if (!this.options.hooks) {
const module = await import(${s(hooks)});
Expand All @@ -114,6 +113,12 @@ export class Server {
externalFetch: module.externalFetch || fetch
};
}
}

async respond(request, options = {}) {
if (!(request instanceof Request)) {
throw new Error('The first argument to server.respond must be a Request object. See https://github.com/sveltejs/kit/pull/3384 for details');
}

return respond(request, this.options, options);
}
Expand Down
8 changes: 7 additions & 1 deletion packages/kit/src/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,13 @@ function kit() {

const child = fork(
script,
[vite_config.build.outDir, results_path, manifest_path, '' + verbose],
[
vite_config.build.outDir,
results_path,
manifest_path,
'' + verbose,
JSON.stringify({ ...env.private, ...env.public })
],
{
stdio: 'inherit'
}
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/vite/preview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export async function preview(vite, vite_config, svelte_config) {
});

const server = new Server(manifest);
server.init({
await server.init({
env: loadEnv(vite_config.mode, process.cwd(), '')
});

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/src/hooks.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from 'fs';
import cookie from 'cookie';
import { sequence } from '../../../../src/hooks';
import { sequence } from '@sveltejs/kit/hooks';

/** @type {import('@sveltejs/kit').HandleError} */
export const handleError = ({ event, error }) => {
Expand Down
5 changes: 5 additions & 0 deletions packages/kit/test/prerendering/basics/.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
PRIVATE_STATIC="accessible to server-side code/replaced at build time"
PRIVATE_DYNAMIC="accessible to server-side code/evaluated at run time"

PUBLIC_STATIC="accessible anywhere/replaced at build time"
PUBLIC_DYNAMIC="accessible anywhere/evaluated at run time"
1 change: 1 addition & 0 deletions packages/kit/test/prerendering/basics/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
!.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { PRIVATE_STATIC } from '$env/static/private';
import { env } from '$env/dynamic/private';

export function load() {
return {
PRIVATE_STATIC,
PRIVATE_DYNAMIC: env.PRIVATE_DYNAMIC
};
}
13 changes: 13 additions & 0 deletions packages/kit/test/prerendering/basics/src/routes/env/+page.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
import { PUBLIC_STATIC } from '$env/static/public';
import { env } from '$env/dynamic/public';

/** @type {import('./$types').PageData} */
export let data;
</script>

<p id="static-private">PRIVATE_STATIC: {data.PRIVATE_STATIC}</p>
<p id="dynamic-private">PRIVATE_DYNAMIC: {data.PRIVATE_DYNAMIC}</p>

<p id="static-public">PUBLIC_STATIC: {PUBLIC_STATIC}</p>
<p id="dynamic-public">PUBLIC_DYNAMIC: {env.PUBLIC_DYNAMIC}</p>
15 changes: 15 additions & 0 deletions packages/kit/test/prerendering/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,19 @@ test('respects config.prerender.origin', () => {
assert.ok(content.includes('<h2>http://example.com</h2>'));
});

test('$env - includes environment variables', () => {
const content = read('env.html');

assert.match(
content,
/.*PRIVATE_STATIC: accessible to server-side code\/replaced at build time.*/gs
);
assert.match(
content,
/.*PRIVATE_DYNAMIC: accessible to server-side code\/evaluated at run time.*/gs
);
assert.match(content, /.*PUBLIC_STATIC: accessible anywhere\/replaced at build time.*/gs);
assert.match(content, /.*PUBLIC_DYNAMIC: accessible anywhere\/evaluated at run time.*/gs);
});

test.run();
2 changes: 1 addition & 1 deletion packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export interface ResolveOptions {

export class Server {
constructor(manifest: SSRManifest);
init(options: ServerInitOptions): void;
init(options: ServerInitOptions): Promise<void>;
respond(request: Request, options: RequestOptions): Promise<Response>;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export interface ImportNode {
}

export class InternalServer extends Server {
init(options: ServerInitOptions): void;
init(options: ServerInitOptions): Promise<void>;
respond(
request: Request,
options: RequestOptions & {
Expand Down