Skip to content

Commit

Permalink
chore: source maps for preprocessors + tests (#10459)
Browse files Browse the repository at this point in the history
Add source map merging for preprocessors and get tests passing.

- fixed some issues around the `sources` array where they weren't calculated relative to the input correctly
- adjusted some CSS tests because due to our own CSS parser the AST is less granular, so there are less mappings now. Don't think this is a problem, but worth thinking about
- removed enableSourcemap but only log a warning, the reason this was introduced was to mitigate a bug in Vite which occured when having the source map inlined into the SSR'd output. Since SSR doesn't contain inlined CSS anymore (and if it did, we would omit the source map) the reason for which it was introduced no longer exists
- files without js mapping in it have no source mappings yet (originally added in Svelte 4 for #6092)
  • Loading branch information
dummdidumm authored Feb 15, 2024
1 parent 4b274dd commit f8ff2b6
Show file tree
Hide file tree
Showing 102 changed files with 926 additions and 1,358 deletions.
5 changes: 5 additions & 0 deletions .changeset/silly-laws-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: add proper source map support
30 changes: 20 additions & 10 deletions packages/svelte/src/compiler/css/Stylesheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import MagicString from 'magic-string';
import { walk } from 'zimmerframe';
import { ComplexSelector } from './Selector.js';
import { hash } from './utils.js';
// import compiler_warnings from '../compiler_warnings.js';
// import { extract_ignores_above_position } from '../utils/extract_svelte_ignore.js';
import { create_attribute } from '../phases/nodes.js'; // TODO move this
import { merge_with_preprocessor_map } from '../utils/mapped_code.js';

const regex_css_browser_prefix = /^-((webkit)|(moz)|(o)|(ms))-/;
const regex_name_boundary = /^[\s,;}]$/;
Expand Down Expand Up @@ -337,7 +336,7 @@ export class Stylesheet {
/** @type {import('#compiler').Style | null} */
ast;

/** @type {string} */
/** @type {string} Path of Svelte file the CSS is in */
filename;

/** @type {boolean} */
Expand Down Expand Up @@ -471,20 +470,23 @@ export class Stylesheet {
}

/**
* @param {string} file
* @param {string} source
* @param {boolean} dev
* @param {import('#compiler').ValidatedCompileOptions} options
*/
render(file, source, dev) {
render(source, options) {
// TODO neaten this up
if (!this.ast) throw new Error('Unexpected error');

const code = new MagicString(source);

// Generate source mappings for the style sheet nodes we have.
// Note that resolution is a bit more coarse than in Svelte 4 because
// our own CSS AST is not as detailed with regards to the node values.
walk(/** @type {import('#compiler').Css.Node} */ (this.ast), null, {
_: (node) => {
_: (node, { next }) => {
code.addSourcemapLocation(node.start);
code.addSourcemapLocation(node.end);
next();
}
});

Expand All @@ -495,19 +497,27 @@ export class Stylesheet {
code.remove(0, this.ast.content.start);

for (const child of this.children) {
child.prune(code, dev);
child.prune(code, options.dev);
}

code.remove(/** @type {number} */ (this.ast.content.end), source.length);

return {
const css = {
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,
file
file: options.cssOutputFilename || this.filename
})
};
merge_with_preprocessor_map(css, options, css.map.sources[0]);
if (options.dev && options.css === 'injected' && css.code) {
css.code += `\n/*# sourceMappingURL=${css.map.toUrl()} */`;
}

return css;
}

/** @param {import('../phases/types.js').ComponentAnalysis} analysis */
Expand Down
17 changes: 16 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import full_char_code_at from './utils/full_char_code_at.js';
import { error } from '../../errors.js';
import { create_fragment } from './utils/create.js';
import read_options from './read/options.js';
import { getLocator } from 'locate-character';

const regex_position_indicator = / \(\d+:\d+\)$/;

Expand Down Expand Up @@ -41,13 +42,16 @@ export class Parser {
/** @type {LastAutoClosedTag | undefined} */
last_auto_closed_tag;

locate;

/** @param {string} template */
constructor(template) {
if (typeof template !== 'string') {
throw new TypeError('Template must be a string');
}

this.template = template.trimEnd();
this.locate = getLocator(this.template, { offsetLine: 1 });

let match_lang;

Expand Down Expand Up @@ -133,6 +137,18 @@ export class Parser {
}
}

/**
* offset -> line/column
* @param {number} start
* @param {number} end
*/
get_location(start, end) {
return {
start: /** @type {import('locate-character').Location_1} */ (this.locate(start)),
end: /** @type {import('locate-character').Location_1} */ (this.locate(end))
};
}

current() {
return this.stack[this.stack.length - 1];
}
Expand Down Expand Up @@ -297,7 +313,6 @@ export class Parser {
*/
export function parse(template) {
const parser = new Parser(template);

return parser.root;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
Loading

0 comments on commit f8ff2b6

Please sign in to comment.