fix: account for sourcemap in meta info (#8778)

We need to use a different method for getting the meta info because `locate` is used to help construct the source map that references the preprocessed Svelte file. If we would now add source maps to that `locate` function it would go the the original source directly which means skipping potentially intermediate source maps which we would need in other situations. Sadly we can't map the character offset because for that we would need to the original source contents which we don't have in this context.

fixes #8360
closes #8362
pull/8772/head
Simon H 2 years ago committed by GitHub
parent 5702142d9e
commit ef1b98f9d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: account for preprocessor source maps when calculating meta info

@ -12,7 +12,7 @@
}
},
{
"files": ["README.md", "packages/*/README.md"],
"files": ["README.md", "packages/*/README.md", "**/package.json"],
"options": {
"useTabs": false,
"tabWidth": 2

@ -103,6 +103,7 @@
"dependencies": {
"@ampproject/remapping": "^2.2.1",
"@jridgewell/sourcemap-codec": "^1.4.15",
"@jridgewell/trace-mapping": "^0.3.18",
"acorn": "^8.8.2",
"aria-query": "^5.2.1",
"axobject-query": "^3.2.1",
@ -135,4 +136,4 @@
"typescript": "^5.0.4",
"vitest": "^0.31.1"
}
}
}

@ -1,3 +1,4 @@
import { TraceMap, originalPositionFor } from '@jridgewell/trace-mapping';
import { walk } from 'estree-walker';
import { getLocator } from 'locate-character';
import { reserved, is_valid } from '../utils/names.js';
@ -142,9 +143,20 @@ export default class Component {
/** @type {string} */
file;
/** @type {(c: number) => { line: number; column: number }} */
/**
* Use this for stack traces. It is 1-based and acts on pre-processed sources.
* Use `meta_locate` for metadata on DOM elements.
* @type {(c: number) => { line: number; column: number }}
*/
locate;
/**
* Use this for metadata on DOM elements. It is 1-based and acts on sources that have not been pre-processed.
* Use `locate` for source mappings.
* @type {(c: number) => { line: number; column: number }}
*/
meta_locate;
/** @type {import('./nodes/Element.js').default[]} */
elements = [];
@ -199,7 +211,25 @@ export default class Component {
.replace(process.cwd(), '')
.replace(regex_leading_directory_separator, '')
: compile_options.filename);
// line numbers in stack trace frames are 1-based. source maps are 0-based
this.locate = getLocator(this.source, { offsetLine: 1 });
/** @type {TraceMap | null | undefined} initialise lazy because only used in dev mode */
let tracer;
this.meta_locate = (c) => {
/** @type {{ line: number, column: number }} */
let location = this.locate(c);
if (tracer === undefined) {
// @ts-expect-error - fix the type of CompileOptions.sourcemap
tracer = compile_options.sourcemap ? new TraceMap(compile_options.sourcemap) : null;
}
if (tracer) {
// originalPositionFor returns 1-based lines like locator
location = originalPositionFor(tracer, location);
}
return location;
};
// styles
this.stylesheet = new Stylesheet({
source,

@ -62,9 +62,20 @@ export default class Renderer {
/** @type {import('estree').Identifier} */
file_var;
/** @type {(c: number) => { line: number; column: number }} */
/**
* Use this for stack traces. It is 1-based and acts on pre-processed sources.
* Use `meta_locate` for metadata on DOM elements.
* @type {(c: number) => { line: number; column: number }}
*/
locate;
/**
* Use this for metadata on DOM elements. It is 1-based and acts on sources that have not been pre-processed.
* Use `locate` for source mappings.
* @type {(c: number) => { line: number; column: number }}
*/
meta_locate;
/**
* @param {import('../Component.js').default} component
* @param {import('../../interfaces.js').CompileOptions} options
@ -73,6 +84,7 @@ export default class Renderer {
this.component = component;
this.options = options;
this.locate = component.locate; // TODO messy
this.meta_locate = component.meta_locate; // TODO messy
this.file_var = options.dev && this.component.get_unique_name('file');
component.vars
.filter((v) => !v.hoistable || (v.export_name && !v.module))

@ -585,9 +585,11 @@ export default class ElementWrapper extends Wrapper {
);
}
if (renderer.options.dev) {
const loc = renderer.locate(this.node.start);
const loc = renderer.meta_locate(this.node.start);
block.chunks.hydrate.push(
b`@add_location(${this.var}, ${renderer.file_var}, ${loc.line - 1}, ${loc.column}, ${
// TODO this.node.start isn't correct if there's a source map. But since we don't know how the
// original source file looked, there's not much we can do.
this.node.start
});`
);

@ -0,0 +1,33 @@
import MagicString from 'magic-string';
import * as path from 'node:path';
// fake preprocessor by doing transforms on the source
const str = new MagicString(
`<script>
type Foo = 'foo';
let foo = 'foo';
</script>
<h1>{foo}</h1>
`.replace(/\r\n/g, '\n')
);
str.remove(8, 26); // remove line type Foo = ...
str.remove(55, 56); // remove whitespace before <h1>
export default {
compileOptions: {
dev: true,
sourcemap: str.generateMap({ hires: true })
},
test({ assert, target }) {
const h1 = target.querySelector('h1');
assert.deepEqual(h1.__svelte_meta.loc, {
file: path.relative(process.cwd(), path.resolve(__dirname, 'main.svelte')),
line: 5, // line 4 in main.svelte, but that's the preprocessed code, the original code is above in the fake preprocessor
column: 1, // line 0 in main.svelte, but that's the preprocessed code, the original code is above in the fake preprocessor
char: 38 // TODO this is wrong but we can't backtrace it due to limitations, see add_location function usage comment for more info
});
}
};

@ -56,6 +56,9 @@ importers:
'@jridgewell/sourcemap-codec':
specifier: ^1.4.15
version: 1.4.15
'@jridgewell/trace-mapping':
specifier: ^0.3.18
version: 0.3.18
acorn:
specifier: ^8.8.2
version: 8.8.2

Loading…
Cancel
Save