Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 10 additions & 8 deletions packages/app-utils/src/prerender/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,27 @@ import { dirname, resolve as resolve_path } from 'path';
import { parse, resolve, URLSearchParams } from 'url';
import { mkdirp } from '../files';
import { render } from '../render';
import { RouteManifest } from '../types';
import { PageResponse, RouteManifest } from '../types';

function clean_html(html) {
function clean_html(html: string) {
return html
.replace(/<!\[CDATA\[[\s\S]*?\]\]>/gm, '')
.replace(/(<script[\s\S]*?>)[\s\S]*?<\/script>/gm, '$1</' + 'script>')
.replace(/(<style[\s\S]*?>)[\s\S]*?<\/style>/gm, '$1</' + 'style>')
.replace(/<!--[\s\S]*?-->/gm, '');
}

function get_href(attrs) {
function get_href(attrs: string) {
const match = /href\s*=\s*(?:"(.*?)"|'(.*?)'|([^\s>]*))/.exec(attrs);
return match && (match[1] || match[2] || match[3]);
}

function get_src(attrs) {
function get_src(attrs: string) {
const match = /src\s*=\s*(?:"(.*?)"|'(.*?)'|([^\s>]*))/.exec(attrs);
return match && (match[1] || match[2] || match[3]);
}

function get_srcset_urls(attrs) {
function get_srcset_urls(attrs: string) {
const results = [];
// Note that the srcset allows any ASCII whitespace, including newlines.
const match = /srcset\s*=\s*(?:"(.*?)"|'(.*?)'|([^\s>]*))/s.exec(attrs);
Expand Down Expand Up @@ -130,9 +130,11 @@ export async function prerender({
log.error(`${rendered.status} ${path}`);
}

if (rendered.dependencies) {
for (const path in rendered.dependencies) {
const result = rendered.dependencies[path];
const dependencies = (rendered as PageResponse).dependencies;

if (dependencies) {
for (const path in dependencies) {
const result = dependencies[path];
const response_type = Math.floor(result.status / 100);

const is_html = result.headers['content-type'] === 'text/html';
Expand Down
2 changes: 1 addition & 1 deletion packages/app-utils/src/render/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import render_page from './page';
import render_route from './route';
import { EndpointResponse, IncomingRequest, PageResponse, RenderOptions } from '../types';

function md5(body) {
function md5(body: string) {
return createHash('md5').update(body).digest('hex');
}

Expand Down
17 changes: 11 additions & 6 deletions packages/app-utils/src/render/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,27 @@ import devalue from 'devalue';
import fetch, { Response } from 'node-fetch';
import * as mime from 'mime';
import { render } from './index';
import { IncomingRequest, RenderOptions, PageManifest } from '../types';
import { IncomingRequest, RenderOptions, PageManifest, EndpointResponse, PageResponse, Headers } from '../types';

const noop = () => {};

type FetchOpts = {
method?: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'HEAD' | 'OPTIONS';
headers?: Record<string, string>;
headers?: Headers;
body?: any;
};

export default async function render_page(
request: IncomingRequest,
context: any,
options: RenderOptions
) {
let redirected;
): Promise<{
status: number,
body?: string,
headers?: Headers,
dependencies?: Record< string, EndpointResponse | PageResponse>
}> {
let redirected: { status: number; location?: string; headers: Headers };
let preload_error;

const page: PageManifest = options.manifest.pages.find(page => page.pattern.test(request.path));
Expand Down Expand Up @@ -53,10 +58,10 @@ export default async function render_page(
l++;
});

const dependencies = {};
const dependencies: Record<string, EndpointResponse | PageResponse> = {};

const preload_context = {
redirect: (status, location) => {
redirect: (status: number, location: string) => {
if (redirected && (redirected.status !== status || redirected.location !== location)) {
throw new Error(`Conflicting redirects`);
}
Expand Down
10 changes: 7 additions & 3 deletions packages/app-utils/src/render/route.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { IncomingRequest, RenderOptions, EndpointManifest } from '../types';
import { IncomingRequest, RenderOptions, EndpointManifest, Headers } from '../types';

export default function render_route(
request: IncomingRequest,
context: any,
options: RenderOptions
) {
): Promise<{
status: number,
body: string,
headers?: Headers
}> {
const route: EndpointManifest = options.manifest.endpoints.find(route => route.pattern.test(request.path));
if (!route) return;

Expand Down Expand Up @@ -53,7 +57,7 @@ export default function render_route(
});
}

function lowercase_keys(obj) {
function lowercase_keys(obj: Record<string, any>) {
const clone = {};
for (const key in obj) {
clone[key.toLowerCase()] = obj[key];
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.DS_Store
/node_modules
/dist
/assets/client.*
/assets/client.*
/client/**/*.d.ts
15 changes: 15 additions & 0 deletions packages/kit/client/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
/* Generates the typings for client code */
"compilerOptions": {
"resolveJsonModule": true,
"module": "esnext",
"moduleResolution": "node",
"target": "ES6",
"esModuleInterop": true,
"emitDeclarationOnly": true,
"declaration": true,
"outDir": "."
},
"include": ["../src/client/**/*", "../src/ambient.d.ts"],
"lib": ["ES2020", "dom", "node"]
}
5 changes: 3 additions & 2 deletions packages/kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@
},
"files": [
"assets",
"dist"
"dist",
"client"
],
"scripts": {
"dev": "rollup -cw",
"build": "rollup -c",
"prepare": "npm run build",
"prepare": "npm run build && cd client && tsc",
"prepublishOnly": "npm run build"
}
}
11 changes: 11 additions & 0 deletions packages/kit/src/ambient.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
declare module "MANIFEST" {
export const components: any[];
export const routes: any[];
export const Layout: any;
export const ErrorComponent: any;
}

declare module "ROOT" {
const root: any;
export default root;
}
2 changes: 1 addition & 1 deletion packages/kit/src/api/dev/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default function loader(snowpack: SnowpackDevServer): Loader {
let data: string;

try {
const result = await snowpack.loadUrl(url, {isSSR: true, encoding: 'utf-8'});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snowpack's typings show it should be utf8 rather than utf-8

const result = await snowpack.loadUrl(url, {isSSR: true, encoding: 'utf8'});
data = result.contents;
} catch (err) {
throw new Error(`Failed to load ${url}: ${err.message}`);
Expand Down
4 changes: 3 additions & 1 deletion packages/kit/src/api/dev/sourcemap_stacktrace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ export function sourcemap_stacktrace(stack: string) {
return input;
}

const consumer = new SourceMapConsumer(raw_sourcemap);
// TODO: according to typings, this code cannot work;
// the constructor returns a promise that needs to be awaited
const consumer = new (SourceMapConsumer as any)(raw_sourcemap);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this whole file doesn't seem to be used anyway (copied from sapper). can we just delete it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps this is because Sapper uses 0.6.1 of source-map. I don't see a version specified in Kit except for source-map: 0.5.7 and source-map: 0.7.3 in pnpm-lock.yaml

before deleting this file we might want to test what happens if an error is thrown server-side in the production app. will the stacktrace be readable (i.e. sourcemapped) or will it be on the minimized code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, it's a newer version; then the code most likely does indeed not work. It's non-trivial to change the code to be async, that's why I didn't do it. Should we downgrade in the meantime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the stack traces: in dev mode, if I throw an exception in Nav.svelte, I get this on the server side:

Error: testing stacktrace
    at eval (/_app/components/Nav.js:15:8)
    at Object.$$render (/web_modules/svelte/internal.js:1343:22)

On the client side, it correctly says /Nav.svelte as location.

On a production build, the client-side stack trace from an error I throw in an on:click looks like this:

Uncaught Error: testing stacktraces
    at HTMLButtonElement.<anonymous> (Nav.js:268)
(anonymous) @ Nav.js:268

The server-side stack traces look like:

Error: testing stacktrace
    at fn (/Users/andreasehrencrona/projects/svelte/kit/examples/hn.svelte.dev/.svelte/build/unoptimized/server/_app/components/Nav.js:13:8)

Is this worth a ticket? I'm not sure what expectations we have; neither of the stack traces is terrible, but it would of course be better if it mentioned the correct source file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we had to stick to 0.6.1 in Sapper because we hadn't figured out how to support newer versions yet

Yeah, it seems worth a ticket to source map the production server-side trace just as Sapper does today

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, created #76

const pos = consumer.originalPositionFor({
line: Number(line),
column: Number(column),
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import relative from 'require-relative';
import sade from 'sade';
import colors from 'kleur';
import * as pkg from '../package.json';
import { ReadyEvent } from './interfaces';
import { ReadyEvent, SvelteAppConfig } from './interfaces';

let config;
let config: SvelteAppConfig;

try {
config = relative('./svelte.config.js', process.cwd());
Expand Down
12 changes: 12 additions & 0 deletions packages/kit/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"resolveJsonModule": true,
"module": "esnext",
"moduleResolution": "node",
"target": "ES6",
"esModuleInterop": true,
"noEmit": true
},
"include": ["src/**/*"],
"lib": ["ES2020", "dom", "node"]
}
1 change: 1 addition & 0 deletions packages/snowpack-config/snowpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = {
'src/setup': '/_app/setup'
},
alias: {
'@sveltejs/kit': '/_app/main',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean the user has to build once to make TS happy because else the typings don't exist yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this configuration doesn't affect your development tools (this is the run-time module resolution). The (dev) dependency @sveltejs/kit already exists; so the development tools will look inside it for the typings and find them in the client/index.d.ts.

(It won't be able to resolve to the source code since it's only copied into place (in .svelte/build during the build)

$components: './src/components'
}
};