From 0f10f9fe2a10363ca81aa6f396f2831db642b841 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 11 Dec 2020 20:48:19 -0500 Subject: [PATCH 1/9] cache module object, not just exports --- packages/kit/src/api/dev/loader.js | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/kit/src/api/dev/loader.js b/packages/kit/src/api/dev/loader.js index f24b68e71192..2edfb0be3334 100644 --- a/packages/kit/src/api/dev/loader.js +++ b/packages/kit/src/api/dev/loader.js @@ -53,27 +53,24 @@ export default function loader(snowpack, config) { return {}; } - if (cache.has(url)) return cache.get(url); + if (cache.has(url)) return cache.get(url).then(module => module.exports); - 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((result) => initialize_module(url, result, url_stack.concat(url))) .catch((e) => { cache.delete(url); throw e; }); - cache.set(url, exports); - return exports; + cache.set(url, promise); + return promise.then(module => module.exports); } - 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 = {}; + const module = { ...loaded, exports: {} }; const args = [ { @@ -91,19 +88,19 @@ export default function loader(snowpack, config) { }, { name: names.exports, - value: exports + value: module.exports }, { name: names.__export, value: (name, get) => { - Object.defineProperty(exports, name, { get }); + Object.defineProperty(module.exports, name, { get }); } }, { name: names.__export_all, value: (mod) => { for (const name in mod) { - Object.defineProperty(exports, name, { + Object.defineProperty(module.exports, name, { get: () => mod[name] }); } @@ -132,7 +129,7 @@ export default function loader(snowpack, config) { fn(...args.map((d) => d.value)); - return exports; + return module; } return (url) => load(url, []); From e64dc90da18de0b813e895c4a12415a09bdfa9d7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 11 Dec 2020 21:26:25 -0500 Subject: [PATCH 2/9] simplify cache invalidation --- packages/kit/src/api/dev/loader.js | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/packages/kit/src/api/dev/loader.js b/packages/kit/src/api/dev/loader.js index 2edfb0be3334..58e795112e4e 100644 --- a/packages/kit/src/api/dev/loader.js +++ b/packages/kit/src/api/dev/loader.js @@ -1,5 +1,4 @@ import { URL } from 'url'; -import { resolve, relative } from 'path'; import { transform } from './transform'; // This function makes it possible to load modules from the 'server' @@ -30,17 +29,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) { @@ -146,11 +141,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; - }, {}); -} From 42d3d06de2143389a2a805a9d67ae41023d69040 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 11 Dec 2020 23:50:45 -0500 Subject: [PATCH 3/9] get sourcemap stacktraces working for eval errors in SSR --- packages/kit/package.json | 4 +- packages/kit/src/api/dev/loader.js | 38 ++++++++++++++----- .../kit/src/api/dev/sourcemap_stacktrace.js | 36 +++++------------- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/packages/kit/package.json b/packages/kit/package.json index 98f94db9157e..6ab255b71aef 100644 --- a/packages/kit/package.json +++ b/packages/kit/package.json @@ -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", @@ -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" diff --git a/packages/kit/src/api/dev/loader.js b/packages/kit/src/api/dev/loader.js index 58e795112e4e..ab64df47fb50 100644 --- a/packages/kit/src/api/dev/loader.js +++ b/packages/kit/src/api/dev/loader.js @@ -1,4 +1,6 @@ +import { existsSync, readFileSync } from 'fs'; import { URL } from 'url'; +import { sourcemap_stacktrace } from './sourcemap_stacktrace'; import { transform } from './transform'; // This function makes it possible to load modules from the 'server' @@ -48,24 +50,24 @@ export default function loader(snowpack, config) { return {}; } - if (cache.has(url)) return cache.get(url).then(module => module.exports); + if (cache.has(url)) return cache.get(url); const promise = snowpack .loadUrl(url, { isSSR: true, encoding: 'utf8' }) - .then((result) => initialize_module(url, result, url_stack.concat(url))) + .then((loaded) => initialize_module(url, loaded, url_stack.concat(url))) .catch((e) => { cache.delete(url); throw e; }); cache.set(url, promise); - return promise.then(module => module.exports); + return promise; } async function initialize_module(url, loaded, url_stack) { const { code, deps, names } = transform(loaded.contents); - const module = { ...loaded, exports: {} }; + const exports = {}; const args = [ { @@ -83,19 +85,19 @@ export default function loader(snowpack, config) { }, { name: names.exports, - value: module.exports + value: exports }, { name: names.__export, value: (name, get) => { - Object.defineProperty(module.exports, name, { get }); + Object.defineProperty(exports, name, { get }); } }, { name: names.__export_all, value: (mod) => { for (const name in mod) { - Object.defineProperty(module.exports, name, { + Object.defineProperty(exports, name, { get: () => mod[name] }); } @@ -122,9 +124,27 @@ 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' }); + return contents; + } catch { + // fail gracefully + } + }); + + throw e; + } - return module; + return exports; } return (url) => load(url, []); diff --git a/packages/kit/src/api/dev/sourcemap_stacktrace.js b/packages/kit/src/api/dev/sourcemap_stacktrace.js index c6a54b86f204..02934274bc03 100644 --- a/packages/kit/src/api/dev/sourcemap_stacktrace.js +++ b/packages/kit/src/api/dev/sourcemap_stacktrace.js @@ -1,4 +1,3 @@ -import fs from 'fs'; import path from 'path'; import { SourceMapConsumer } from 'source-map'; @@ -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) => { @@ -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)) { @@ -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; @@ -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 }); @@ -97,7 +83,5 @@ export async function sourcemap_stacktrace(stack) { } ); - file_cache.clear(); - return (await Promise.all(stack.split('\n').map(replace))).join('\n'); } From d6046c841248221974fa2922e33e7e0e35db2722 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Dec 2020 14:44:58 -0500 Subject: [PATCH 4/9] sourcemap stack traces in build output --- packages/kit/src/renderer/page.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/renderer/page.js b/packages/kit/src/renderer/page.js index 6d0f50c0ef26..138175c31161 100644 --- a/packages/kit/src/renderer/page.js +++ b/packages/kit/src/renderer/page.js @@ -1,5 +1,5 @@ import devalue from 'devalue'; -import { createReadStream, existsSync } from 'fs'; +import { createReadStream, existsSync, fstat, readFileSync } from 'fs'; import * as mime from 'mime'; import fetch, { Response } from 'node-fetch'; import { writable } from 'svelte/store'; @@ -265,6 +265,12 @@ export default async function render_page(request, context, options) { } catch (error) { try { const status = error.status || 500; + error.stack = await sourcemap_stacktrace(error.stack, address => { + // TODO this won't work in all environments + if (existsSync(address)) { + return readFileSync(address, 'utf-8'); + } + }); return await get_response({ request, @@ -279,7 +285,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: {} }; } From c0fcb502fef1f54ef0f9c04874b5df8af05a69a9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Dec 2020 15:49:36 -0500 Subject: [PATCH 5/9] update lockfile --- packages/kit/src/renderer/page.js | 1 + pnpm-lock.yaml | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/renderer/page.js b/packages/kit/src/renderer/page.js index 138175c31161..fb4e35402ac3 100644 --- a/packages/kit/src/renderer/page.js +++ b/packages/kit/src/renderer/page.js @@ -267,6 +267,7 @@ export default async function render_page(request, context, options) { const status = error.status || 500; error.stack = await sourcemap_stacktrace(error.stack, address => { // TODO this won't work in all environments + // TODO sourcemaps are broken due to https://github.com/snowpackjs/snowpack/discussions/1941 if (existsSync(address)) { return readFileSync(address, 'utf-8'); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 00a2ccef9c49..4924b2dbeab3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -174,6 +174,7 @@ importers: sade: 1.7.4 scorta: 1.0.0 snowpack: 3.0.0-rc.1 + source-map: 0.7.3 devDependencies: '@types/node': 14.11.10 '@types/rimraf': 3.0.0 @@ -191,7 +192,6 @@ importers: 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 @@ -4133,6 +4133,7 @@ packages: resolution: integrity: sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g== /source-map/0.7.3: + dev: false engines: node: '>= 8' resolution: From 595d0099f3285412e4866d39ef29d33a636b927e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Dec 2020 19:31:43 -0500 Subject: [PATCH 6/9] remove unused import --- packages/kit/src/renderer/page.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/renderer/page.js b/packages/kit/src/renderer/page.js index fb4e35402ac3..759caff3de4b 100644 --- a/packages/kit/src/renderer/page.js +++ b/packages/kit/src/renderer/page.js @@ -1,5 +1,5 @@ import devalue from 'devalue'; -import { createReadStream, existsSync, fstat, readFileSync } from 'fs'; +import { createReadStream, existsSync, readFileSync } from 'fs'; import * as mime from 'mime'; import fetch, { Response } from 'node-fetch'; import { writable } from 'svelte/store'; From 407a298b9427755d56be49f6ff7e9144b6008219 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Dec 2020 19:32:52 -0500 Subject: [PATCH 7/9] formatting --- packages/kit/src/renderer/page.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/renderer/page.js b/packages/kit/src/renderer/page.js index 759caff3de4b..57273f4e01e7 100644 --- a/packages/kit/src/renderer/page.js +++ b/packages/kit/src/renderer/page.js @@ -265,7 +265,7 @@ export default async function render_page(request, context, options) { } catch (error) { try { const status = error.status || 500; - error.stack = await sourcemap_stacktrace(error.stack, address => { + error.stack = await sourcemap_stacktrace(error.stack, (address) => { // TODO this won't work in all environments // TODO sourcemaps are broken due to https://github.com/snowpackjs/snowpack/discussions/1941 if (existsSync(address)) { From d0938066d0a1384a479be0aa2f8b6f8eab9ff537 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Dec 2020 19:34:02 -0500 Subject: [PATCH 8/9] remove prod sourcemapped stacktraces for now --- packages/kit/src/renderer/page.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/kit/src/renderer/page.js b/packages/kit/src/renderer/page.js index 57273f4e01e7..2c57b1390abc 100644 --- a/packages/kit/src/renderer/page.js +++ b/packages/kit/src/renderer/page.js @@ -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; @@ -265,13 +264,8 @@ export default async function render_page(request, context, options) { } catch (error) { try { const status = error.status || 500; - error.stack = await sourcemap_stacktrace(error.stack, (address) => { - // TODO this won't work in all environments - // TODO sourcemaps are broken due to https://github.com/snowpackjs/snowpack/discussions/1941 - if (existsSync(address)) { - return readFileSync(address, 'utf-8'); - } - }); + + // TODO sourcemapped stacktrace? https://github.com/sveltejs/kit/pull/266 return await get_response({ request, From 2bce4af63ea261a88958046f53e860ae0911cc59 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 12 Dec 2020 19:34:20 -0500 Subject: [PATCH 9/9] remove unused import --- packages/kit/src/renderer/page.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/renderer/page.js b/packages/kit/src/renderer/page.js index 2c57b1390abc..5fd3e53716e4 100644 --- a/packages/kit/src/renderer/page.js +++ b/packages/kit/src/renderer/page.js @@ -1,5 +1,5 @@ import devalue from 'devalue'; -import { createReadStream, existsSync, readFileSync } from 'fs'; +import { createReadStream, existsSync } from 'fs'; import * as mime from 'mime'; import fetch, { Response } from 'node-fetch'; import { writable } from 'svelte/store';