Consistently return mutable position objects from source map consumer

Summary: Previously, `SectionsConsumer` would reuse the frozen `EMPTY_POSITION` object when querying positions that don't belong to any section. With this diff we'll clone it instead. There is an `originalPositionFor` call site in `metro-symbolicate` which relies on adding properties to the position object, which is possible in the `source-map` API we're emulating, so here we explicitly make this supported. Reviewed By: MichaReiser Differential Revision: D26166651 fbshipit-source-id: ee1e0aaefbbb59a0fd5f6bf24d94ffe11a3a84d6
facebook · Feb 2, 2021 · fab9af9 · fab9af9
1 parent 2cc5aa8
commit fab9af9
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 5 deletions.
diff --git a/packages/metro-source-map/src/Consumer/SectionsConsumer.js b/packages/metro-source-map/src/Consumer/SectionsConsumer.js
@@ -53,7 +53,7 @@ class SectionsConsumer extends AbstractConsumer implements IConsumer {
     const [generatedOffset, consumer] =
       this._consumerForPosition(generatedPosition) || [];
     if (!consumer) {
-      return EMPTY_POSITION;
+      return {...EMPTY_POSITION};
     }
     return consumer.originalPositionFor(
       subtractOffsetFromPosition(generatedPosition, generatedOffset),

diff --git a/packages/metro-source-map/src/Consumer/types.flow.js b/packages/metro-source-map/src/Consumer/types.flow.js
@@ -15,10 +15,10 @@ import type {Number0, Number1} from 'ob1';
 export type {IterationOrder, LookupBias};
 export type GeneratedOffset = {|+lines: Number0, +columns: Number0|};
 export type SourcePosition = {
-  +source: ?string,
-  +line: ?Number1,
-  +column: ?Number0,
-  +name: ?string,
+  source: ?string,
+  line: ?Number1,
+  column: ?Number0,
+  name: ?string,
   ...
 };
 export type GeneratedPosition = {

diff --git a/packages/metro-source-map/src/__tests__/Consumer-test.js b/packages/metro-source-map/src/__tests__/Consumer-test.js
@@ -37,6 +37,29 @@ describe('basic maps', () => {
       `);
     });
 
+    test('empty position is a mutable object', () => {
+      const consumer = new Consumer({
+        version: 3,
+        mappings: '',
+        names: [],
+        sources: [],
+      });
+      const empty1 = consumer.originalPositionFor({
+        line: add1(0),
+        column: add0(0),
+      });
+      const empty2 = consumer.originalPositionFor({
+        line: add1(0),
+        column: add0(0),
+      });
+      expect(empty1).not.toBe(empty2);
+      expect(() => {
+        empty1.name = 'foo';
+        // $FlowIgnore[prop-missing]
+        empty1.someProp = 'bar';
+      }).not.toThrow();
+    });
+
     test('single full mapping', () => {
       const consumer = new Consumer({
         version: 3,
@@ -259,6 +282,27 @@ describe('indexed (sectioned) maps', () => {
       `);
     });
 
+    test('empty position is a mutable object', () => {
+      const consumer = new Consumer({
+        version: 3,
+        sections: [],
+      });
+      const empty1 = consumer.originalPositionFor({
+        line: add1(0),
+        column: add0(0),
+      });
+      const empty2 = consumer.originalPositionFor({
+        line: add1(0),
+        column: add0(0),
+      });
+      expect(empty1).not.toBe(empty2);
+      expect(() => {
+        empty1.name = 'foo';
+        // $FlowIgnore[prop-missing]
+        empty1.someProp = 'bar';
+      }).not.toThrow();
+    });
+
     test('section per column', () => {
       const consumer = new Consumer({
         version: 3,