From a75e292f57a51dad0e476bfe7009fcc1a32233d9 Mon Sep 17 00:00:00 2001 From: Daniel McLaughlin Date: Mon, 20 Sep 2021 02:38:37 -0700 Subject: [PATCH] Make line counts reflect all line endings (#691) 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: https://github.com/facebook/metro/pull/691 Reviewed By: feedthejim Differential Revision: D30607069 Pulled By: motiz88 fbshipit-source-id: c83d5cd87de40b204e9b7ade08ce831360751bf6 --- packages/metro-source-map/src/source-map.js | 7 ++++--- .../src/__tests__/index-test.js | 21 +++++++++++++++++++ packages/metro/src/lib/countLines.js | 9 +++----- .../metro/src/shared/output/RamBundle/util.js | 6 +----- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/packages/metro-source-map/src/source-map.js b/packages/metro-source-map/src/source-map.js index 53cc4e56f7..2ef546543c 100644 --- a/packages/metro-source-map/src/source-map.js +++ b/packages/metro-source-map/src/source-map.js @@ -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, diff --git a/packages/metro-transform-worker/src/__tests__/index-test.js b/packages/metro-transform-worker/src/__tests__/index-test.js index 0f88ee7aba..54b08b697c 100644 --- a/packages/metro-transform-worker/src/__tests__/index-test.js +++ b/packages/metro-transform-worker/src/__tests__/index-test.js @@ -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, + ); +}); diff --git a/packages/metro/src/lib/countLines.js b/packages/metro/src/lib/countLines.js index 8805740ef1..ddb5cdc554 100644 --- a/packages/metro/src/lib/countLines.js +++ b/packages/metro/src/lib/countLines.js @@ -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; diff --git a/packages/metro/src/shared/output/RamBundle/util.js b/packages/metro/src/shared/output/RamBundle/util.js index 5e3cc4860d..6e3498695a 100644 --- a/packages/metro/src/shared/output/RamBundle/util.js +++ b/packages/metro/src/shared/output/RamBundle/util.js @@ -12,6 +12,7 @@ const invariant = require('invariant'); +import countLines from '../../../lib/countLines'; import type {ModuleGroups, ModuleTransportLike} from '../../types.flow'; import type { IndexMap, @@ -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 = '',