Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some issues with preprocess source maps #5754

Merged
merged 11 commits into from
Dec 16, 2020
31 changes: 20 additions & 11 deletions src/compiler/preprocess/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,18 @@ function parse_attributes(str: string) {
return attrs;
}

function get_file_basename(filename: string) {
return filename.split(/[/\\]/).pop();
}

interface Replacement {
offset: number;
length: number;
replacement: StringWithSourcemap;
}

async function replace_async(
filename: string,
file_basename: string,
source: string,
get_location: ReturnType<typeof getLocator>,
re: RegExp,
Expand All @@ -73,13 +77,13 @@ async function replace_async(
)) {
// content = unchanged source characters before the replaced segment
const content = StringWithSourcemap.from_source(
filename, source.slice(last_end, offset), get_location(last_end));
file_basename, source.slice(last_end, offset), get_location(last_end));
out.concat(content).concat(replacement);
last_end = offset + length;
}
// final_content = unchanged source characters after last replaced segment
const final_content = StringWithSourcemap.from_source(
filename, source.slice(last_end), get_location(last_end));
file_basename, source.slice(last_end), get_location(last_end));
return out.concat(final_content);
}

Expand Down Expand Up @@ -160,7 +164,7 @@ function decoded_sourcemap_from_generator(generator: any) {
* Convert a preprocessor output and its leading prefix and trailing suffix into StringWithSourceMap
*/
function get_replacement(
filename: string,
file_basename: string,
offset: number,
get_location: ReturnType<typeof getLocator>,
original: string,
Expand All @@ -171,9 +175,9 @@ function get_replacement(

// Convert the unchanged prefix and suffix to StringWithSourcemap
const prefix_with_map = StringWithSourcemap.from_source(
filename, prefix, get_location(offset));
file_basename, prefix, get_location(offset));
const suffix_with_map = StringWithSourcemap.from_source(
filename, suffix, get_location(offset + prefix.length + original.length));
file_basename, suffix, get_location(offset + prefix.length + original.length));

// Convert the preprocessed code and its sourcemap to a StringWithSourcemap
let decoded_map: DecodedSourceMap;
Expand All @@ -186,7 +190,9 @@ function get_replacement(
// import decoded sourcemap from mozilla/source-map/SourceMapGenerator
decoded_map = decoded_sourcemap_from_generator(decoded_map);
}
sourcemap_add_offset(decoded_map, get_location(offset + prefix.length));
// offset only segments pointing at original component source
const source_index = decoded_map.sources.findIndex(source => source === file_basename);
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
sourcemap_add_offset(decoded_map, get_location(offset + prefix.length), source_index);
}
const processed_with_map = StringWithSourcemap.from_processed(processed.code, decoded_map);

Expand All @@ -203,6 +209,9 @@ export default async function preprocess(
const filename = (options && options.filename) || preprocessor.filename; // legacy
const dependencies = [];

// preprocess source must be relative to itself
const file_basename = filename && get_file_basename(filename);

const preprocessors = preprocessor
? Array.isArray(preprocessor) ? preprocessor : [preprocessor]
: [];
Expand Down Expand Up @@ -246,13 +255,13 @@ export default async function preprocess(
: /<!--[^]*?-->|<script(\s[^]*?)?(?:>([^]*?)<\/script>|\/>)/gi;

const res = await replace_async(
filename,
file_basename,
source,
get_location,
tag_regex,
async (match, attributes = '', content = '', offset) => {
const no_change = () => StringWithSourcemap.from_source(
filename, match, get_location(offset));
file_basename, match, get_location(offset));
if (!attributes && !content) {
return no_change();
}
Expand All @@ -268,7 +277,7 @@ export default async function preprocess(

if (!processed) return no_change();
if (processed.dependencies) dependencies.push(...processed.dependencies);
return get_replacement(filename, offset, get_location, content, processed, `<${tag_name}${attributes}>`, `</${tag_name}>`);
return get_replacement(file_basename, offset, get_location, content, processed, `<${tag_name}${attributes}>`, `</${tag_name}>`);
}
);
source = res.string;
Expand All @@ -285,7 +294,7 @@ export default async function preprocess(

// Combine all the source maps for each preprocessor function into one
const map: RawSourceMap = combine_sourcemaps(
filename,
file_basename,
sourcemap_list
);

Expand Down
43 changes: 28 additions & 15 deletions src/compiler/utils/string_with_sourcemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,22 @@ function last_line_length(s: string) {

// mutate map in-place
export function sourcemap_add_offset(
map: DecodedSourceMap, offset: SourceLocation
map: DecodedSourceMap, offset: SourceLocation, source_index: number
) {
if (map.mappings.length == 0) return map;
// shift columns in first line
const segment_list = map.mappings[0];
for (let segment = 0; segment < segment_list.length; segment++) {
const seg = segment_list[segment];
if (seg[3]) seg[3] += offset.column;
}
if (map.mappings.length == 0 || source_index < 0) return;
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
// shift lines
for (let line = 0; line < map.mappings.length; line++) {
const segment_list = map.mappings[line];
for (let segment = 0; segment < segment_list.length; segment++) {
const seg = segment_list[segment];
if (seg[2]) seg[2] += offset.line;
// shift only segments that belong to component source file
if (seg.length >= 4 && seg[1] === source_index) {
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
// shift columns pointing at the first line
if (seg[2] === 0) {
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
seg[3] += offset.column;
}
seg[2] += offset.line;
}
}
}
}
Expand Down Expand Up @@ -97,6 +98,9 @@ export class StringWithSourcemap {
return this;
}

// compute last line length before mutating
const column_offset = last_line_length(this.string);

this.string += other.string;

const m1 = this.map;
Expand All @@ -117,24 +121,24 @@ export class StringWithSourcemap {
const segment_list = m2.mappings[line];
for (let segment = 0; segment < segment_list.length; segment++) {
const seg = segment_list[segment];
if (seg[1]) seg[1] = new_source_idx[seg[1]];
if (seg[4]) seg[4] = new_name_idx[seg[4]];
if (seg.length > 1) seg[1] = new_source_idx[seg[1]];
if (seg.length > 4) seg[4] = new_name_idx[seg[4]];
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
}
}
} else if (sources_idx_changed) {
for (let line = 0; line < m2.mappings.length; line++) {
const segment_list = m2.mappings[line];
for (let segment = 0; segment < segment_list.length; segment++) {
const seg = segment_list[segment];
if (seg[1]) seg[1] = new_source_idx[seg[1]];
if (seg.length > 1) seg[1] = new_source_idx[seg[1]];
}
}
} else if (names_idx_changed) {
for (let line = 0; line < m2.mappings.length; line++) {
const segment_list = m2.mappings[line];
for (let segment = 0; segment < segment_list.length; segment++) {
const seg = segment_list[segment];
if (seg[4]) seg[4] = new_name_idx[seg[4]];
if (seg.length > 4) seg[4] = new_name_idx[seg[4]];
}
}
}
Expand All @@ -146,7 +150,6 @@ export class StringWithSourcemap {
// 2. first line of second map
// columns of 2 must be shifted

const column_offset = last_line_length(this.string);
if (m2.mappings.length > 0 && column_offset > 0) {
const first_line = m2.mappings[0];
for (let i = 0; i < first_line.length; i++) {
Expand All @@ -164,7 +167,17 @@ export class StringWithSourcemap {
}

static from_processed(string: string, map?: DecodedSourceMap): StringWithSourcemap {
if (map) return new StringWithSourcemap(string, map);
if (map) {
// ensure that count of source map mappings lines
// is equal to count of generated code lines
// (some tools may produce less)
const missing_lines = string.split('\n').length - map.mappings.length;
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
for (let i = 0; i < missing_lines; i++) {
map.mappings.push([]);
}
return new StringWithSourcemap(string, map);
}

if (string == '') return new StringWithSourcemap();
map = { version: 3, names: [], sources: [], mappings: [] };

Expand Down
79 changes: 78 additions & 1 deletion test/sourcemaps/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,81 @@
import MagicString from 'magic-string';
import * as assert from 'assert';
import { getLocator } from 'locate-character';
import MagicString, { Bundle } from 'magic-string';

type AssertMappedParameters = {
code: string;
filename?: string;
input: string | ReturnType<typeof getLocator>;
input_code?: string;
preprocessed: any;
};

export function assert_mapped(
{ code, filename, input, input_code, preprocessed }: AssertMappedParameters
) {
const locate_input = typeof input === 'function' ? input : getLocator(input);
if (filename === undefined) filename = 'input.svelte';
if (input_code === undefined) input_code = code;

const source_loc = locate_input(input_code);
assert.notEqual(
source_loc,
undefined,
`failed to locate "${input_code}" in "${filename}"`
);

const transformed_loc = preprocessed.locate_1(code);
assert.notEqual(
transformed_loc,
undefined,
`failed to locate "${code}" in transformed "${filename}"`
);

assert.deepEqual(
preprocessed.mapConsumer.originalPositionFor(transformed_loc),
{
source: filename,
name: null,
line: source_loc.line + 1,
column: source_loc.column
},
`incorrect mappings for "${input_code}" in "${filename}"`
);
}

export function assert_not_located(
code: string,
locate: ReturnType<typeof getLocator>,
filename = 'input.svelte'
) {
assert.equal(
locate(code),
undefined,
`located "${code}" that should be removed from ${filename}`
);
}

export function magic_string_bundle(
inputs: Array<{ code: string | MagicString, filename: string }>,
filename = 'bundle.js',
separator = '\n'
) {
const bundle = new Bundle({ separator });
inputs.forEach(({ code, filename }) => {
bundle.addSource({
filename,
content: typeof code === 'string' ? new MagicString(code) : code
});
});
return {
code: bundle.toString(),
map: bundle.generateMap({
source: filename,
hires: true,
includeContent: false
})
};
}

export function magic_string_preprocessor_result(filename: string, src: MagicString) {
return {
Expand Down
24 changes: 24 additions & 0 deletions test/sourcemaps/samples/external/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { magic_string_bundle } from '../../helpers';

export const COMMON = ':global(html) { height: 100%; }\n';

// TODO: removing '\n' breaks test
// - _actual.svelte.map looks correct
// - _actual.css.map adds reference to </style> on input.svelte
// - Most probably caused by bug in current magic-string version (fixed in 0.25.7)
export const STYLES = '.awesome { color: orange; }\n';

export default {
css_map_sources: ['common.scss', 'styles.scss'],
js_map_sources: [],
preprocess: [
{
style: () => {
return magic_string_bundle([
{ filename: 'common.scss', code: COMMON },
{ filename: 'styles.scss', code: STYLES }
]);
}
}
]
};
3 changes: 3 additions & 0 deletions test/sourcemaps/samples/external/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<style lang="scss" src="./styles.scss"></style>

<div class="awesome">Divs ftw!</div>
26 changes: 26 additions & 0 deletions test/sourcemaps/samples/external/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { assert_mapped } from '../../helpers';
import { COMMON, STYLES } from './_config';

export function test({ input, preprocessed }) {
// Transformed script, main file
assert_mapped({
filename: 'input.svelte',
code: 'Divs ftw!',
input: input.locate,
preprocessed
});

// External files
assert_mapped({
filename: 'common.scss',
code: 'height: 100%;',
input: COMMON,
preprocessed
});
assert_mapped({
filename: 'styles.scss',
code: 'color: orange;',
input: STYLES,
preprocessed
});
}
22 changes: 22 additions & 0 deletions test/sourcemaps/samples/script-and-style/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import MagicString from 'magic-string';
import { magic_string_preprocessor_result } from '../../helpers';

export default {
js_map_sources: [
'input.svelte'
],
preprocess: [
{
script: ({ content, filename }) => {
const s = new MagicString(content);
s.prepend('// This script code is approved\n');
return magic_string_preprocessor_result(filename, s);
},
style: ({ content, filename }) => {
const s = new MagicString(content);
s.prepend('/* This style code is approved */\n');
return magic_string_preprocessor_result(filename, s);
}
}
]
};
19 changes: 19 additions & 0 deletions test/sourcemaps/samples/script-and-style/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<script>
import { onMount } from 'svelte';

let count = 0;

onMount(() => {
const id = setInterval(() => count++, 1000);
return () => clearInterval(id);
});
</script>

<style>
h1 {
color: orange;
}
</style>

<h1>Hello world!</h1>
<div>Counter value: {count}</div>
Loading