Skip to content

Commit

Permalink
Make line counts reflect all line endings (#691)
Browse files Browse the repository at this point in the history
Summary:
**Summary**

Metro included a few functions to count source lines, some of which didn't correctly count all [line terminators specified in the ECMAScript standard](https://262.ecma-international.org/12.0/#sec-line-terminators). This PR replaces those implementations with the correct one found in `packages/metro/src/shared/output/RamBundle/util.js`.

The user-facing issue which led me to this was that importing a file with CRLF line endings caused `/symbolicate` to return incorrect mappings for all files after that one in the bundle. Metro-transform-worker was double-counting the line endings and getExplodedSourceMap was carrying that erroneous offset through.

**Test plan**

There's a new test for a variety of line endings, and the several existing tests that exercise countLines pass without modification.

Pull Request resolved: #691

Reviewed By: feedthejim

Differential Revision: D30607069

Pulled By: motiz88

fbshipit-source-id: c83d5cd87de40b204e9b7ade08ce831360751bf6
  • Loading branch information
danielsmc authored and facebook-github-bot committed Sep 20, 2021
1 parent 1bde8b9 commit a75e292
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 14 deletions.
7 changes: 4 additions & 3 deletions packages/metro-source-map/src/source-map.js
Expand Up @@ -277,9 +277,10 @@ function addMapping(generator, mapping, carryOver) {
}
}

function countLines(string) {
return string.split('\n').length;
}
const newline = /\r\n?|\n|\u2028|\u2029/g;

const countLines = (string: string): number =>
(string.match(newline) || []).length + 1;

module.exports = {
BundleBuilder,
Expand Down
21 changes: 21 additions & 0 deletions packages/metro-transform-worker/src/__tests__/index-test.js
Expand Up @@ -600,3 +600,24 @@ it('skips minification in Hermes canary transform profile', async () => {
});"
`);
});

it('counts all line endings correctly', async () => {
const transformStr = str =>
Transformer.transform(baseConfig, '/root', 'local/file.js', str, {
dev: false,
minify: false,
type: 'module',
});

const differentEndingsResult = await transformStr(
'one\rtwo\r\nthree\nfour\u2028five\u2029six',
);

const standardEndingsResult = await transformStr(
'one\ntwo\nthree\nfour\nfive\nsix',
);

expect(differentEndingsResult.output[0].data.lineCount).toEqual(
standardEndingsResult.output[0].data.lineCount,
);
});
9 changes: 3 additions & 6 deletions packages/metro/src/lib/countLines.js
Expand Up @@ -10,12 +10,9 @@

'use strict';

const nullthrows = require('nullthrows');
const newline = /\r\n?|\n|\u2028|\u2029/g;

const reLine = /^/gm;

function countLines(s: string): number {
return nullthrows(s.match(reLine)).length;
}
const countLines = (string: string): number =>
(string.match(newline) || []).length + 1;

module.exports = countLines;
6 changes: 1 addition & 5 deletions packages/metro/src/shared/output/RamBundle/util.js
Expand Up @@ -12,6 +12,7 @@

const invariant = require('invariant');

import countLines from '../../../lib/countLines';
import type {ModuleGroups, ModuleTransportLike} from '../../types.flow';
import type {
IndexMap,
Expand All @@ -20,11 +21,6 @@ import type {
BasicSourceMap,
} from 'metro-source-map';

const newline = /\r\n?|\n|\u2028|\u2029/g;
// fastest implementation
const countLines = (string: string): number =>
(string.match(newline) || []).length + 1;

function lineToLineSourceMap(
source: string,
filename: string = '',
Expand Down

0 comments on commit a75e292

Please sign in to comment.