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
4 changes: 2 additions & 2 deletions packages/kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"rollup-plugin-terser": "^7.0.2",
"sade": "^1.7.4",
"scorta": "^1.0.0",
"snowpack": "^3.0.0-rc.1"
"snowpack": "^3.0.0-rc.1",
"source-map": "^0.7.3"
},
"devDependencies": {
"@types/node": "^14.11.10",
Expand All @@ -29,7 +30,6 @@
"require-relative": "^0.8.7",
"rimraf": "^3.0.2",
"sirv": "^1.0.7",
"source-map": "^0.7.3",
"source-map-support": "^0.5.19",
"svelte": "^3.29.0",
"tiny-glob": "^0.2.8"
Expand Down
58 changes: 31 additions & 27 deletions packages/kit/src/api/dev/loader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync, readFileSync } from 'fs';
import { URL } from 'url';
import { resolve, relative } from 'path';
import { sourcemap_stacktrace } from './sourcemap_stacktrace';
import { transform } from './transform';

// This function makes it possible to load modules from the 'server'
Expand Down Expand Up @@ -30,17 +31,13 @@ export default function loader(snowpack, config) {
if (dependents) dependents.forEach(invalidate_all);
};

const absolute_mount = map_keys(config.mount, resolve);

snowpack.onFileChange(({ filePath }) => {
for (const path in absolute_mount) {
if (filePath.startsWith(path)) {
const relative_path = relative(path, filePath.replace(/\.\w+?$/, '.js'));
const url = resolve(absolute_mount[path].url, relative_path);

cache.forEach(async (promise, url) => {
const module = await promise;
if (module.originalFileLoc === filePath) {
invalidate_all(url);
}
}
});
});

async function load(url, url_stack) {
Expand All @@ -55,23 +52,20 @@ export default function loader(snowpack, config) {

if (cache.has(url)) return cache.get(url);

const exports = snowpack
const promise = snowpack
.loadUrl(url, { isSSR: true, encoding: 'utf8' })
.catch((err) => {
throw new Error(`Failed to load ${url}: ${err.message}`);
})
.then((result) => initialize_module(url, result.contents, url_stack.concat(url)))
.then((loaded) => initialize_module(url, loaded, url_stack.concat(url)))
.catch((e) => {
cache.delete(url);
throw e;
});

cache.set(url, exports);
return exports;
cache.set(url, promise);
return promise;
}

async function initialize_module(url, data, url_stack) {
const { code, deps, names } = transform(data);
async function initialize_module(url, loaded, url_stack) {
const { code, deps, names } = transform(loaded.contents);

const exports = {};

Expand Down Expand Up @@ -130,7 +124,25 @@ export default function loader(snowpack, config) {

const fn = new Function(...args.map((d) => d.name), `${code}\n//# sourceURL=${url}`);

fn(...args.map((d) => d.value));
try {
fn(...args.map((d) => d.value));
} catch (e) {
e.stack = await sourcemap_stacktrace(e.stack, async (address) => {
if (existsSync(address)) {
// it's a filepath
return readFileSync(address, 'utf-8');
}

try {
const { contents } = await snowpack.loadUrl(address, { isSSR: true, encoding: 'utf8' });
Copy link
Member

Choose a reason for hiding this comment

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

I thought sourcemap file URLs were paths from the project root on the file system. It might be helpful to add a comment explaining when this would occur. I'm also wondering about the fact that this is needed here and not in page.js

Copy link
Member Author

Choose a reason for hiding this comment

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

Per the Resolving sources section of the standard,

If the sources are not absolute URLs after prepending of the “sourceRoot”, the sources are resolved relative to the SourceMap (like resolving script src in a html document).

In other words, to find the original file for a give segment, find the index into the sources array, and do path.resolve(sourcemap_file, source). There's some ambiguity here because information gets lost between the filesystem and a webserver, but this is the most reliable way to reconstruct a filepath.

I'm also wondering about the fact that this is needed here and not in page.js

loadUrl is invoked for things other than rendering pages (e.g. setup, endpoints)

Copy link
Member

@benmccann benmccann Dec 12, 2020

Choose a reason for hiding this comment

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

Why would we need to call loadUrl for endpoints and not pages?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's called for both

return contents;
} catch {
// fail gracefully
}
});

throw e;
}

return exports;
}
Expand All @@ -149,11 +161,3 @@ function load_node(source) {
}
});
}

function map_keys(object, map) {
return Object.entries(object).reduce((new_object, [k, v]) => {
new_object[map(k)] = v;

return new_object;
}, {});
}
36 changes: 10 additions & 26 deletions packages/kit/src/api/dev/sourcemap_stacktrace.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import fs from 'fs';
import path from 'path';
import { SourceMapConsumer } from 'source-map';

Expand All @@ -11,22 +10,6 @@ function get_sourcemap_url(contents) {
return undefined;
}

const file_cache = new Map();

function get_file_contents(file_path) {
if (file_cache.has(file_path)) {
return file_cache.get(file_path);
}

try {
const data = fs.readFileSync(file_path, 'utf8');
file_cache.set(file_path, data);
return data;
} catch {
return undefined;
}
}

async function replace_async(str, regex, asyncFn) {
const promises = [];
str.replace(regex, (match, ...args) => {
Expand All @@ -37,21 +20,23 @@ async function replace_async(str, regex, asyncFn) {
return str.replace(regex, () => data.shift());
}

export async function sourcemap_stacktrace(stack) {
// TODO does Snowpack compose sourcemaps, or will this only undo
// the last in a series of transformations?
export async function sourcemap_stacktrace(stack, load_contents) {
const replace = (line) =>
replace_async(
line,
/^ {4}at (?:(.+?)\s+\()?(?:(.+?):(\d+)(?::(\d+))?)\)?/,
async (input, var_name, file_path, line_number, column) => {
if (!file_path) return input;
async (input, var_name, address, line, column) => {
if (!address) return input;

const contents = get_file_contents(file_path);
const contents = await load_contents(address);
if (!contents) return input;

const sourcemap_url = get_sourcemap_url(contents);
if (!sourcemap_url) return input;

let dir = path.dirname(file_path);
let dir = path.dirname(address);
let sourcemap_data;

if (/^data:application\/json[^,]+base64,/.test(sourcemap_url)) {
Expand All @@ -63,7 +48,7 @@ export async function sourcemap_stacktrace(stack) {
}
} else {
const sourcemap_path = path.resolve(dir, sourcemap_url);
const data = get_file_contents(sourcemap_path);
const data = await load_contents(sourcemap_path);

if (!data) return input;

Expand All @@ -81,8 +66,9 @@ export async function sourcemap_stacktrace(stack) {
// TODO: according to typings, this code cannot work;
// the constructor returns a promise that needs to be awaited
const consumer = await new SourceMapConsumer(raw_sourcemap);

const pos = consumer.originalPositionFor({
line: Number(line_number),
line: Number(line),
column: Number(column),
bias: SourceMapConsumer.LEAST_UPPER_BOUND
});
Expand All @@ -97,7 +83,5 @@ export async function sourcemap_stacktrace(stack) {
}
);

file_cache.clear();

return (await Promise.all(stack.split('\n').map(replace))).join('\n');
}
5 changes: 3 additions & 2 deletions packages/kit/src/renderer/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import fetch, { Response } from 'node-fetch';
import { writable } from 'svelte/store';
import { parse, resolve, URLSearchParams } from 'url';
import { render } from './index';
import { sourcemap_stacktrace } from '../api/dev/sourcemap_stacktrace';

async function get_response({ request, options, session, page, status = 200, error }) {
let redirected;
Expand Down Expand Up @@ -266,6 +265,8 @@ export default async function render_page(request, context, options) {
try {
const status = error.status || 500;

// TODO sourcemapped stacktrace? https://github.com/sveltejs/kit/pull/266

return await get_response({
request,
options,
Expand All @@ -279,7 +280,7 @@ export default async function render_page(request, context, options) {
return {
status: 500,
headers: {},
body: await sourcemap_stacktrace(error.stack), // TODO probably not in prod?
body: error.stack, // TODO probably not in prod?
dependencies: {}
};
}
Expand Down
3 changes: 2 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.