Skip to content

Commit ac3aa04

Browse files
petebacondarwinjosephperrott
authored andcommitted
refactor(compiler-cli): avoid free-standing FileSystem functions (angular#39006)
These free standing functions rely upon the "current" `FileSystem`, but it is safer to explicitly pass the `FileSystem` into functions or classes that need it. PR Close angular#39006
1 parent 23e2188 commit ac3aa04

File tree

3 files changed

+47
-40
lines changed

3 files changed

+47
-40
lines changed

packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import {removeComments, removeMapFileComments} from 'convert-source-map';
99
import {decode, encode, SourceMapMappings, SourceMapSegment} from 'sourcemap-codec';
1010

11-
import {AbsoluteFsPath, dirname, relative} from '../../file_system';
11+
import {AbsoluteFsPath, FileSystem} from '../../file_system';
1212

1313
import {RawSourceMap} from './raw_source_map';
1414
import {compareSegments, offsetSegment, SegmentMarker} from './segment_marker';
@@ -38,7 +38,9 @@ export class SourceFile {
3838
/** Whether this source file's source map was inline or external. */
3939
readonly inline: boolean,
4040
/** Any source files referenced by the raw source map associated with this source file. */
41-
readonly sources: (SourceFile|null)[]) {
41+
readonly sources: (SourceFile|null)[],
42+
private fs: FileSystem,
43+
) {
4244
this.contents = removeSourceMapComments(contents);
4345
this.startOfLinePositions = computeStartOfLinePositions(this.contents);
4446
this.flattenedMappings = this.flattenMappings();
@@ -75,11 +77,11 @@ export class SourceFile {
7577
mappings[line].push(mappingArray);
7678
}
7779

78-
const sourcePathDir = dirname(this.sourcePath);
80+
const sourcePathDir = this.fs.dirname(this.sourcePath);
7981
const sourceMap: RawSourceMap = {
8082
version: 3,
81-
file: relative(sourcePathDir, this.sourcePath),
82-
sources: sources.map(sf => relative(sourcePathDir, sf.sourcePath)),
83+
file: this.fs.relative(sourcePathDir, this.sourcePath),
84+
sources: sources.map(sf => this.fs.relative(sourcePathDir, sf.sourcePath)),
8385
names,
8486
mappings: encode(mappings),
8587
sourcesContent: sources.map(sf => sf.contents),

packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file_loader.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
import {commentRegex, fromComment, mapFileCommentRegex} from 'convert-source-map';
99

10-
import {absoluteFrom, AbsoluteFsPath, FileSystem} from '../../file_system';
10+
import {AbsoluteFsPath, FileSystem} from '../../file_system';
1111
import {Logger} from '../../logging';
1212

1313
import {RawSourceMap} from './raw_source_map';
@@ -83,7 +83,7 @@ export class SourceFileLoader {
8383
inline = mapAndPath.mapPath === null;
8484
}
8585

86-
return new SourceFile(sourcePath, contents, map, inline, sources);
86+
return new SourceFile(sourcePath, contents, map, inline, sources, this.fs);
8787
} catch (e) {
8888
this.logger.warn(
8989
`Unable to fully load ${sourcePath} for source-map flattening: ${e.message}`);
@@ -124,7 +124,7 @@ export class SourceFileLoader {
124124
}
125125
}
126126

127-
const impliedMapPath = absoluteFrom(sourcePath + '.map');
127+
const impliedMapPath = this.fs.resolve(sourcePath + '.map');
128128
if (this.fs.exists(impliedMapPath)) {
129129
return {map: this.readRawSourceMap(impliedMapPath), mapPath: impliedMapPath};
130130
}

packages/compiler-cli/src/ngtsc/sourcemaps/test/source_file_spec.ts

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,19 @@
77
*/
88
import {encode} from 'sourcemap-codec';
99

10-
import {absoluteFrom} from '../../file_system';
10+
import {absoluteFrom, FileSystem, getFileSystem} from '../../file_system';
1111
import {runInEachFileSystem} from '../../file_system/testing';
1212
import {RawSourceMap} from '../src/raw_source_map';
1313
import {SegmentMarker} from '../src/segment_marker';
1414
import {computeStartOfLinePositions, ensureOriginalSegmentLinks, extractOriginalSegments, findLastMappingIndexBefore, Mapping, parseMappings, SourceFile} from '../src/source_file';
1515

1616
runInEachFileSystem(() => {
1717
describe('SourceFile and utilities', () => {
18+
let fs: FileSystem;
1819
let _: typeof absoluteFrom;
1920

2021
beforeEach(() => {
22+
fs = getFileSystem();
2123
_ = absoluteFrom;
2224
});
2325

@@ -40,7 +42,7 @@ runInEachFileSystem(() => {
4042
sources: ['a.js'],
4143
version: 3
4244
};
43-
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
45+
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
4446
const mappings = parseMappings(rawSourceMap, [originalSource], [0, 8]);
4547
expect(mappings).toEqual([
4648
{
@@ -71,7 +73,8 @@ runInEachFileSystem(() => {
7173

7274
it('should parse the segments in ascending order of original position from the raw source map',
7375
() => {
74-
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
76+
const originalSource =
77+
new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
7578
const rawSourceMap: RawSourceMap = {
7679
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2]]]),
7780
names: [],
@@ -88,8 +91,8 @@ runInEachFileSystem(() => {
8891
});
8992

9093
it('should create separate arrays for each original source file', () => {
91-
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
92-
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, []);
94+
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
95+
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, [], fs);
9396
const rawSourceMap: RawSourceMap = {
9497
mappings:
9598
encode([[[0, 0, 0, 0], [2, 1, 0, 3], [4, 0, 0, 2], [5, 1, 0, 5], [6, 1, 0, 2]]]),
@@ -313,8 +316,8 @@ runInEachFileSystem(() => {
313316
describe('ensureOriginalSegmentLinks', () => {
314317
it('should add `next` properties to each segment that point to the next segment in the same source file',
315318
() => {
316-
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
317-
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, []);
319+
const sourceA = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
320+
const sourceB = new SourceFile(_('/foo/src/b.js'), '1234567', null, false, [], fs);
318321
const rawSourceMap: RawSourceMap = {
319322
mappings:
320323
encode([[[0, 0, 0, 0], [2, 1, 0, 3], [4, 0, 0, 2], [5, 1, 0, 5], [6, 1, 0, 2]]]),
@@ -336,14 +339,14 @@ runInEachFileSystem(() => {
336339
describe('flattenedMappings', () => {
337340
it('should be an empty array for source files with no source map', () => {
338341
const sourceFile =
339-
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, []);
342+
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, [], fs);
340343
expect(sourceFile.flattenedMappings).toEqual([]);
341344
});
342345

343346
it('should be empty array for source files with no source map mappings', () => {
344347
const rawSourceMap: RawSourceMap = {mappings: '', names: [], sources: [], version: 3};
345348
const sourceFile =
346-
new SourceFile(_('/foo/src/index.js'), 'index contents', rawSourceMap, false, []);
349+
new SourceFile(_('/foo/src/index.js'), 'index contents', rawSourceMap, false, [], fs);
347350
expect(sourceFile.flattenedMappings).toEqual([]);
348351
});
349352

@@ -355,25 +358,26 @@ runInEachFileSystem(() => {
355358
sources: ['a.js'],
356359
version: 3
357360
};
358-
const originalSource = new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, []);
361+
const originalSource =
362+
new SourceFile(_('/foo/src/a.js'), 'abcdefg', null, false, [], fs);
359363
const sourceFile = new SourceFile(
360-
_('/foo/src/index.js'), 'abc123defg', rawSourceMap, false, [originalSource]);
364+
_('/foo/src/index.js'), 'abc123defg', rawSourceMap, false, [originalSource], fs);
361365
expect(removeOriginalSegmentLinks(sourceFile.flattenedMappings))
362366
.toEqual(parseMappings(rawSourceMap, [originalSource], [0, 11]));
363367
});
364368

365369
it('should merge mappings from flattened original source files', () => {
366-
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, []);
367-
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, []);
370+
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, [], fs);
371+
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, [], fs);
368372

369373
const bSourceMap: RawSourceMap = {
370374
mappings: encode([[[0, 1, 0, 0], [1, 0, 0, 0], [4, 1, 0, 1]]]),
371375
names: [],
372376
sources: ['c.js', 'd.js'],
373377
version: 3
374378
};
375-
const bSource =
376-
new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource]);
379+
const bSource = new SourceFile(
380+
_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource], fs);
377381

378382
const aSourceMap: RawSourceMap = {
379383
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
@@ -382,7 +386,7 @@ runInEachFileSystem(() => {
382386
version: 3
383387
};
384388
const aSource =
385-
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource]);
389+
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource], fs);
386390

387391
expect(removeOriginalSegmentLinks(aSource.flattenedMappings)).toEqual([
388392
{
@@ -431,15 +435,16 @@ runInEachFileSystem(() => {
431435
sources: ['c.js'],
432436
version: 3
433437
};
434-
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [null]);
438+
const bSource =
439+
new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [null], fs);
435440
const aSourceMap: RawSourceMap = {
436441
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
437442
names: [],
438443
sources: ['b.js'],
439444
version: 3
440445
};
441446
const aSource =
442-
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource]);
447+
new SourceFile(_('/foo/src/a.js'), 'abdecf', aSourceMap, false, [bSource], fs);
443448

444449
// These flattened mappings are just the mappings from a to b.
445450
// (The mappings to c are dropped since there is no source file to map to.)
@@ -462,23 +467,23 @@ runInEachFileSystem(() => {
462467

463468
describe('renderFlattenedSourceMap()', () => {
464469
it('should convert the flattenedMappings into a raw source-map object', () => {
465-
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123e', null, false, []);
470+
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123e', null, false, [], fs);
466471
const bToCSourceMap: RawSourceMap = {
467472
mappings: encode([[[1, 0, 0, 0], [4, 0, 0, 3], [4, 0, 0, 6], [5, 0, 0, 7]]]),
468473
names: [],
469474
sources: ['c.js'],
470475
version: 3
471476
};
472477
const bSource =
473-
new SourceFile(_('/foo/src/b.js'), 'abcdef', bToCSourceMap, false, [cSource]);
478+
new SourceFile(_('/foo/src/b.js'), 'abcdef', bToCSourceMap, false, [cSource], fs);
474479
const aToBSourceMap: RawSourceMap = {
475480
mappings: encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]]]),
476481
names: [],
477482
sources: ['b.js'],
478483
version: 3
479484
};
480485
const aSource =
481-
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource]);
486+
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource], fs);
482487

483488
const aTocSourceMap = aSource.renderFlattenedSourceMap();
484489
expect(aTocSourceMap.version).toEqual(3);
@@ -493,7 +498,7 @@ runInEachFileSystem(() => {
493498
});
494499

495500
it('should handle mappings that map from lines outside of the actual content lines', () => {
496-
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', null, false, []);
501+
const bSource = new SourceFile(_('/foo/src/b.js'), 'abcdef', null, false, [], fs);
497502
const aToBSourceMap: RawSourceMap = {
498503
mappings: encode([
499504
[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5]],
@@ -506,7 +511,7 @@ runInEachFileSystem(() => {
506511
version: 3
507512
};
508513
const aSource =
509-
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource]);
514+
new SourceFile(_('/foo/src/a.js'), 'abdecf', aToBSourceMap, false, [bSource], fs);
510515

511516
const aTocSourceMap = aSource.renderFlattenedSourceMap();
512517
expect(aTocSourceMap.version).toEqual(3);
@@ -522,13 +527,13 @@ runInEachFileSystem(() => {
522527
describe('getOriginalLocation()', () => {
523528
it('should return null for source files with no flattened mappings', () => {
524529
const sourceFile =
525-
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, []);
530+
new SourceFile(_('/foo/src/index.js'), 'index contents', null, false, [], fs);
526531
expect(sourceFile.getOriginalLocation(1, 1)).toEqual(null);
527532
});
528533

529534
it('should return offset locations in multiple flattened original source files', () => {
530-
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, []);
531-
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, []);
535+
const cSource = new SourceFile(_('/foo/src/c.js'), 'bcd123', null, false, [], fs);
536+
const dSource = new SourceFile(_('/foo/src/d.js'), 'aef', null, false, [], fs);
532537

533538
const bSourceMap: RawSourceMap = {
534539
mappings: encode([
@@ -542,8 +547,8 @@ runInEachFileSystem(() => {
542547
sources: ['c.js', 'd.js'],
543548
version: 3
544549
};
545-
const bSource =
546-
new SourceFile(_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource]);
550+
const bSource = new SourceFile(
551+
_('/foo/src/b.js'), 'abcdef', bSourceMap, false, [cSource, dSource], fs);
547552

548553
const aSourceMap: RawSourceMap = {
549554
mappings: encode([
@@ -560,7 +565,7 @@ runInEachFileSystem(() => {
560565
version: 3
561566
};
562567
const aSource =
563-
new SourceFile(_('/foo/src/a.js'), 'abde\n cf', aSourceMap, false, [bSource]);
568+
new SourceFile(_('/foo/src/a.js'), 'abde\n cf', aSourceMap, false, [bSource], fs);
564569

565570
// Line 0
566571
expect(aSource.getOriginalLocation(0, 0)) // a
@@ -592,8 +597,8 @@ runInEachFileSystem(() => {
592597
});
593598

594599
it('should return offset locations across multiple lines', () => {
595-
const originalSource =
596-
new SourceFile(_('/foo/src/original.js'), 'abcdef\nghijk\nlmnop', null, false, []);
600+
const originalSource = new SourceFile(
601+
_('/foo/src/original.js'), 'abcdef\nghijk\nlmnop', null, false, [], fs);
597602
const generatedSourceMap: RawSourceMap = {
598603
mappings: encode([
599604
[
@@ -614,7 +619,7 @@ runInEachFileSystem(() => {
614619
};
615620
const generatedSource = new SourceFile(
616621
_('/foo/src/generated.js'), 'ABC\nGHIJDEFK\nLMNOP', generatedSourceMap, false,
617-
[originalSource]);
622+
[originalSource], fs);
618623

619624
// Line 0
620625
expect(generatedSource.getOriginalLocation(0, 0)) // A

0 commit comments

Comments
 (0)