From 211a91acdc2574aa07aa81a83f1058d92d283657 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 15 Feb 2024 11:15:38 +0100 Subject: [PATCH] add location in some places for better source maps --- .../src/compiler/phases/1-parse/index.js | 22 ++++++++++++++++++- .../compiler/phases/1-parse/read/context.js | 1 + .../phases/2-analyze/css/Stylesheet.js | 1 + .../phases/3-transform/client/utils.js | 17 +++++++++++++- .../3-transform/client/visitors/template.js | 15 +++++++++---- .../src/compiler/phases/3-transform/index.js | 6 ++++- .../3-transform/server/transform-server.js | 4 ++-- packages/svelte/src/compiler/phases/scope.js | 2 +- packages/svelte/src/compiler/types/index.d.ts | 7 ++++-- .../svelte/src/compiler/types/template.d.ts | 2 +- .../tests/sourcemaps/samples/css/_config.js | 2 -- .../tests/sourcemaps/samples/css/test.js | 17 -------------- .../sourcemaps/samples/each-block/_config.js | 1 - .../sourcemaps/samples/each-block/test.js | 18 --------------- 14 files changed, 64 insertions(+), 51 deletions(-) delete mode 100644 packages/svelte/tests/sourcemaps/samples/css/test.js delete mode 100644 packages/svelte/tests/sourcemaps/samples/each-block/test.js diff --git a/packages/svelte/src/compiler/phases/1-parse/index.js b/packages/svelte/src/compiler/phases/1-parse/index.js index acf96678ff..bb1fc8d418 100644 --- a/packages/svelte/src/compiler/phases/1-parse/index.js +++ b/packages/svelte/src/compiler/phases/1-parse/index.js @@ -133,6 +133,27 @@ export class Parser { } } + /** + * offset -> line/column + * @param {number} start + * @param {number} end + */ + get_location(start, end) { + // this can probably be cached/optimized + const lines_start = this.template.slice(0, start).split('\n'); + const lines_end = this.template.slice(0, end).split('\n'); + return { + start: { + line: lines_start.length, + column: lines_start.at(-1)?.length || 0 + }, + end: { + line: lines_end.length, + column: lines_end.at(-1)?.length || 0 + } + }; + } + current() { return this.stack[this.stack.length - 1]; } @@ -297,7 +318,6 @@ export class Parser { */ export function parse(template) { const parser = new Parser(template); - return parser.root; } diff --git a/packages/svelte/src/compiler/phases/1-parse/read/context.js b/packages/svelte/src/compiler/phases/1-parse/read/context.js index e284af3380..3759dd2049 100644 --- a/packages/svelte/src/compiler/phases/1-parse/read/context.js +++ b/packages/svelte/src/compiler/phases/1-parse/read/context.js @@ -28,6 +28,7 @@ export default function read_pattern(parser) { type: 'Identifier', name, start, + loc: parser.get_location(start, parser.index), end: parser.index, typeAnnotation: annotation }; diff --git a/packages/svelte/src/compiler/phases/2-analyze/css/Stylesheet.js b/packages/svelte/src/compiler/phases/2-analyze/css/Stylesheet.js index 427bb5d06c..f61ad6b70e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/css/Stylesheet.js +++ b/packages/svelte/src/compiler/phases/2-analyze/css/Stylesheet.js @@ -538,6 +538,7 @@ export default class Stylesheet { return { code: code.toString(), map: code.generateMap({ + // include source content; makes it easier/more robust looking up the source map code includeContent: true, // generateMap takes care of calculating source relative to file source: this.filename, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index ee2ed36544..d9351d3a0f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -75,7 +75,7 @@ export function serialize_get_binding(node, state) { } if (binding.expression) { - return binding.expression; + return typeof binding.expression === 'function' ? binding.expression(node) : binding.expression; } if (binding.kind === 'prop') { @@ -521,6 +521,7 @@ function get_hoistable_params(node, context) { } else if ( // If it's a destructured derived binding, then we can extract the derived signal reference and use that. binding.expression !== null && + typeof binding.expression !== 'function' && binding.expression.type === 'MemberExpression' && binding.expression.object.type === 'CallExpression' && binding.expression.object.callee.type === 'Identifier' && @@ -668,3 +669,17 @@ export function should_proxy_or_freeze(node, scope) { } return true; } + +/** + * Port over the location information from the source to the target identifier. + * but keep the target as-is (i.e. a new id is created). + * This ensures esrap can generate accurate source maps. + * @param {import('estree').Identifier} target + * @param {import('estree').Identifier} source + */ +export function with_loc(target, source) { + if (source.loc) { + return { ...target, loc: source.loc }; + } + return target; +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 5bef734b63..c297a04a68 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -17,6 +17,7 @@ import { is_custom_element_node, is_element_node } from '../../../nodes.js'; import * as b from '../../../../utils/builders.js'; import { error } from '../../../../errors.js'; import { + with_loc, function_visitor, get_assignment_value, serialize_get_binding, @@ -2315,14 +2316,20 @@ export const template_visitors = { each_node_meta.contains_group_binding || !node.index ? each_node_meta.index : b.id(node.index); - const item = b.id(each_node_meta.item_name); + const item = each_node_meta.item; const binding = /** @type {import('#compiler').Binding} */ (context.state.scope.get(item.name)); - binding.expression = each_item_is_reactive ? b.call('$.unwrap', item) : item; + binding.expression = (id) => { + const item_with_loc = with_loc(item, id); + return each_item_is_reactive ? b.call('$.unwrap', item_with_loc) : item_with_loc; + }; if (node.index) { const index_binding = /** @type {import('#compiler').Binding} */ ( context.state.scope.get(node.index) ); - index_binding.expression = each_item_is_reactive ? b.call('$.unwrap', index) : index; + index_binding.expression = (id) => { + const index_with_loc = with_loc(index, id); + return each_item_is_reactive ? b.call('$.unwrap', index_with_loc) : index_with_loc; + }; } /** @type {import('estree').Statement[]} */ @@ -2337,7 +2344,7 @@ export const template_visitors = { ) ); } else { - const unwrapped = binding.expression; + const unwrapped = binding.expression(binding.node); const paths = extract_paths(node.context); for (const path of paths) { diff --git a/packages/svelte/src/compiler/phases/3-transform/index.js b/packages/svelte/src/compiler/phases/3-transform/index.js index 38660281e3..628560161e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/index.js +++ b/packages/svelte/src/compiler/phases/3-transform/index.js @@ -48,7 +48,11 @@ export function transform_component(analysis, source, options) { } const js_source_name = get_source_name(options.filename, options.outputFilename, 'input.svelte'); - const js = print(program, { sourceMapSource: js_source_name }); + const js = print(program, { + // include source content; makes it easier/more robust looking up the source map code + sourceMapContent: source, + sourceMapSource: js_source_name + }); merge_with_preprocessor_map(js, options, js_source_name); const css = diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 8112a30c1c..36717f9a9c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -329,7 +329,7 @@ function serialize_get_binding(node, state) { } if (binding.expression) { - return binding.expression; + return typeof binding.expression === 'function' ? binding.expression(node) : binding.expression; } return node; @@ -1311,7 +1311,7 @@ const template_visitors = { const each_node_meta = node.metadata; const collection = /** @type {import('estree').Expression} */ (context.visit(node.expression)); - const item = b.id(each_node_meta.item_name); + const item = each_node_meta.item; const index = each_node_meta.contains_group_binding || !node.index ? each_node_meta.index diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index c4fb1ba493..fd0981151d 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -556,7 +556,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { contains_group_binding: false, array_name: needs_array_deduplication ? state.scope.root.unique('$$array') : null, index: scope.root.unique('$$index'), - item_name: node.context.type === 'Identifier' ? node.context.name : '$$item', + item: node.context.type === 'Identifier' ? node.context : b.id('$$item'), declarations: scope.declarations, references: [...references_within] .map((id) => /** @type {import('#compiler').Binding} */ (state.scope.get(id.name))) diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 83197354e2..e7142cda49 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -285,8 +285,11 @@ export interface Binding { legacy_dependencies: Binding[]; /** Legacy props: the `class` in `{ export klass as class}` */ prop_alias: string | null; - /** If this is set, all references should use this expression instead of the identifier name */ - expression: Expression | null; + /** + * If this is set, all references should use this expression instead of the identifier name. + * If a function is given, it will be called with the identifier at that location and should return the new expression. + */ + expression: Expression | ((id: Identifier) => Expression) | null; /** If this is set, all mutations should use this expression */ mutation: ((assignment: AssignmentExpression, context: Context) => Expression) | null; } diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 26b6ee7ca2..0cbe05c57b 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -378,7 +378,7 @@ export interface EachBlock extends BaseNode { /** Set if something in the array expression is shadowed within the each block */ array_name: Identifier | null; index: Identifier; - item_name: string; + item: Identifier; declarations: Map; /** List of bindings that are referenced within the expression */ references: Binding[]; diff --git a/packages/svelte/tests/sourcemaps/samples/css/_config.js b/packages/svelte/tests/sourcemaps/samples/css/_config.js index 0608a85159..df3c83c703 100644 --- a/packages/svelte/tests/sourcemaps/samples/css/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/css/_config.js @@ -1,7 +1,5 @@ import { test } from '../../test'; export default test({ - skip: true, // TODO no template mappings - client: ['foo'], css: [{ str: '.foo', strGenerated: '.foo.svelte-sg04hs' }] }); diff --git a/packages/svelte/tests/sourcemaps/samples/css/test.js b/packages/svelte/tests/sourcemaps/samples/css/test.js deleted file mode 100644 index 1e0dda1dff..0000000000 --- a/packages/svelte/tests/sourcemaps/samples/css/test.js +++ /dev/null @@ -1,17 +0,0 @@ -export function test({ assert, input, css }) { - const expected = input.locate('.foo'); - - const start = css.locate('.foo'); - - const actual = css.mapConsumer.originalPositionFor({ - line: start.line + 1, - column: start.column - }); - - assert.deepEqual(actual, { - source: 'input.svelte', - name: null, - line: expected.line + 1, - column: expected.column - }); -} diff --git a/packages/svelte/tests/sourcemaps/samples/each-block/_config.js b/packages/svelte/tests/sourcemaps/samples/each-block/_config.js index 4e08caa6ac..600ae7cbef 100644 --- a/packages/svelte/tests/sourcemaps/samples/each-block/_config.js +++ b/packages/svelte/tests/sourcemaps/samples/each-block/_config.js @@ -1,6 +1,5 @@ import { test } from '../../test'; export default test({ - skip: true, // TODO no mapping for bar client: ['foo', 'bar', { str: 'bar', idxGenerated: 1, idxOriginal: 1 }] }); diff --git a/packages/svelte/tests/sourcemaps/samples/each-block/test.js b/packages/svelte/tests/sourcemaps/samples/each-block/test.js deleted file mode 100644 index 7a811a0748..0000000000 --- a/packages/svelte/tests/sourcemaps/samples/each-block/test.js +++ /dev/null @@ -1,18 +0,0 @@ -export function test({ assert, input, js }) { - const start_index = js.code.indexOf('create_main_fragment'); - - const expected = input.locate('each'); - const start = js.locate('length', start_index); - - const actual = js.mapConsumer.originalPositionFor({ - line: start.line + 1, - column: start.column - }); - - assert.deepEqual(actual, { - source: 'input.svelte', - name: null, - line: expected.line + 1, - column: expected.column - }); -}