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

Add applySourceMap #3

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"typescript": "4.6.3"
},
"dependencies": {
"@jridgewell/resolve-uri": "3.1.0",
"@jridgewell/set-array": "^1.0.1",
"@jridgewell/sourcemap-codec": "^1.4.10",
"@jridgewell/trace-mapping": "^0.3.9"
Expand Down
132 changes: 129 additions & 3 deletions src/gen-mapping.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SetArray, put } from '@jridgewell/set-array';
import { SetArray, put, get } from '@jridgewell/set-array';
import { encode } from '@jridgewell/sourcemap-codec';
import { TraceMap, decodedMappings } from '@jridgewell/trace-mapping';
import { TraceMap, decodedMappings, traceSegment } from '@jridgewell/trace-mapping';

import {
COLUMN,
Expand All @@ -10,6 +10,9 @@ import {
NAMES_INDEX,
} from './sourcemap-segment';

import relativeUri from '@jridgewell/resolve-uri/relative';
import resolveUri from '@jridgewell/resolve-uri';

import type { SourceMapInput } from '@jridgewell/trace-mapping';
import type { SourceMapSegment } from './sourcemap-segment';
import type { DecodedSourceMap, EncodedSourceMap, Pos, Mapping } from './types';
Expand Down Expand Up @@ -139,6 +142,18 @@ export let fromMap: (input: SourceMapInput) => GenMapping;
*/
export let allMappings: (map: GenMapping) => Mapping[];

/**
* Applies the mappings of a sub-source-map for a specific source file to the
* source map being generated. Each mapping to the supplied source file is
* rewritten using the supplied source map.
*/
export let applySourceMap: (
map: GenMapping,
sourceMapConsumer: TraceMap,
sourceFile?: string,
sourceMapPath?: string,
) => void;

// This split declaration is only so that terser can elminiate the static initialization block.
let addSegmentInternal: <S extends string | null | undefined>(
skipable: boolean,
Expand Down Expand Up @@ -216,7 +231,11 @@ export class GenMapping {

setSourceContent = (map, source, content) => {
const { _sources: sources, _sourcesContent: sourcesContent } = map;
sourcesContent[put(sources, source)] = content;

const sourceIndex = get(sources, source);
if (sourceIndex !== undefined) {
sourcesContent[sourceIndex] = content;
}
Comment on lines +234 to +238
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not directly related to applySourceMap I've seen while preparing my PR to postcss that setSourceContent accepts to add any source content even for unknown sources.

However this would break if the setSourceContent is called before adding mappings and I don't know if that's needed or not.

source-map-js has a different approach where they store all source contents by Map<path, content> and create the sourcesContent when transforming to an encoded map, effectively dropping the unneeded sources.

What do you think is the best approach?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However this would break if the setSourceContent is called before adding mappings and I don't know if that's needed or not.

Can you describe what you mean? setSourceContent should create a new index, and if a mapping is ever added with that source filename, that index will be used. Sourcemaps mappings don't need to be monotonic to the source locations.

source-map-js has a different approach where they store all source contents by Map<path, content> and create the sourcesContent when transforming to an encoded map, effectively dropping the unneeded sources.

I think this depends on the answer to the previous question. For the general case, I think it's fine to add source content for sources that never receive a mapping (and it keeps the library fast). But I'm not sure of a situation that your code will do that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I'm not sure of a situation that your code will do that.

Yeah sorry I could have elaborated from the start. I have a test case in postcss that has a sourcemap for /root/dir/a.css.
Then we receive a call for setSourceContent('file:///root/dir/a.css' which essentially is the same file, but with a different path.

On one side, there is the fact that the call itself is incorrect, since the path we know is /root/... receiving file:///root won't work. The current code however happily accepts to add the sourceContent for it even if there is no mapping for it.

In the case of source-map-js it accepts the source content, but doesn't output it if there is no mapping, and thus no entry in .sources.

setSourceContent should create a new index, and if a mapping is ever added with that source filename, that index will be used.

I agree with this, but on the other hand, I'm not sure that the map should have a sourceContent for something that's never mapped

For the general case, I think it's fine to add source content for sources that never receive a mapping (and it keeps the library fast).

Agreed, but this could be done in a map, and resolved when doing the toEncodedMap, this would reduce the number of lookups and do it only once when exporting the sourcemap

};

toDecodedMap = (map) => {
Expand Down Expand Up @@ -336,6 +355,113 @@ export class GenMapping {
: [genColumn, sourcesIndex, sourceLine, sourceColumn],
);
};

applySourceMap = (
map: GenMapping,
consumer: TraceMap,
rawSourceFile?: string | null,
sourceMapPath?: string | null,
) => {
let sourceFile = rawSourceFile ?? consumer.file;
if (sourceFile == null) {
throw new Error(
'applySourceMap requires either an explicit source file, ' +
'or the source map\'s "file" property. Both were omitted.',
);
}

if (!sourceMapPath) sourceMapPath = '';
else if (!sourceMapPath.endsWith('/')) sourceMapPath += '/';

const { _mappings: mappings, _sourcesContent: sourcesContent, sourceRoot } = map;
const sources = map._sources.array;
const names = map._names.array;

const {
sources: consumerSources,
sourcesContent: consumerSourcesContent,
names: consumerNames,
} = consumer;

let sourceIndex = get(map._sources, sourceFile);
if (sourceIndex === undefined && sourceRoot) {
// If we couldn't fine the source file and there's a sourceRoot, the sourceFile may have
// been joined with the root already. Try again after making the file relative to the root.
sourceFile = relativeUri(sourceRoot, sourceFile);
sourceIndex = get(map._sources, sourceFile);
}

// If we still can't find the source file, then there's no way we could remap the file with
// the new consumer map.
if (sourceIndex === undefined) return;

// The source file and several names could be replaced by the remapped consumer map. To avoid
// leaving dead entires, we regenerate the both (and the paired sourcesContent).
const newSources = new SetArray();
const newSourcesContent: (string | null)[] = [];
const newNames = new SetArray();

for (let i = 0; i < mappings.length; i++) {
const line = mappings[i];

for (let j = 0; j < line.length; j++) {
const seg = line[j];
if (seg.length === 1) continue;

let traced = null;
if (seg[1] === sourceIndex) {
traced = traceSegment(consumer, seg[2], seg[3]);
}

if (traced == null) {
// Either this segment is mapping a different source file, or we couldn't traced the
// original mapping in the consumer. Either way, we'll keep the original.
const initialIndex = seg[1];
const newSourceIndex = (seg[1] = put(newSources, sources[initialIndex]));
newSourcesContent[newSourceIndex] = sourcesContent[initialIndex];
if (seg.length === 5) seg[4] = put(newNames, names[seg[4]]);
continue;
}

if (traced.length === 1) {
// If the traced mapping points to a sourceless segment, we need to truncate the
// original to also be sourceless.
(seg as number[]).length = 1;
continue;
}
Comment on lines +427 to +431
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the last block that isn't covered by tests.
Since this was introduced by your PR to my PR, I have to admit I'm not sure what case it covers and what input would cover this test. do you have an example ?


let source = consumerSources[traced[1]] || '';
if (sourceMapPath != null) {
source = resolveUri(source, sourceMapPath);
}
if (sourceRoot != null) {
source = relativeUri(sourceRoot, source);
}

const newSourceIndex = put(newSources, source);
newSourcesContent[newSourceIndex] = consumerSourcesContent
? consumerSourcesContent[traced[1]]
: null;

seg[1] = newSourceIndex;
seg[2] = traced[2];
seg[3] = traced[3];

// If the traced segment has a name, prioritize that. If not, we'll take the original's
// name. We can't shorten this into a ternary because we may not have a name at all, and
// we cannot set index 4 if there is no name.
if (traced.length === 5) {
seg[4] = put(newNames, consumerNames[traced[4]]);
} else if (seg.length == 5) {
seg[4] = put(newNames, names[seg[4]]);
}
}
}

map._sources = newSources;
map._sourcesContent = newSourcesContent;
map._names = newNames;
};
}
}

Expand Down
Loading