Generate codeframe positions for JSON5 (#7933)

parcel-bundler · Apr 20, 2022 · 7d92d81 · 7d92d81
1 parent a389964
commit 7d92d81
Show file tree

Hide file tree

Showing 19 changed files with 151 additions and 80 deletions.
diff --git a/flow-libs/json-source-map.js.flow b/flow-libs/json-source-map.js.flow
diff --git a/packages/core/core/package.json b/packages/core/core/package.json
@@ -24,6 +24,7 @@
     "check-ts": "tsc --noEmit index.d.ts"
   },
   "dependencies": {
+    "@mischnic/json-sourcemap": "^0.1.0",
     "@parcel/cache": "2.4.1",
     "@parcel/diagnostic": "2.4.1",
     "@parcel/events": "2.4.1",
@@ -43,7 +44,6 @@
     "clone": "^2.1.1",
     "dotenv": "^7.0.0",
     "dotenv-expand": "^5.1.0",
-    "json-source-map": "^0.6.1",
     "json5": "^2.2.0",
     "msgpackr": "^1.5.4",
     "nullthrows": "^1.1.1",

diff --git a/packages/core/core/src/requests/EntryRequest.js b/packages/core/core/src/requests/EntryRequest.js
@@ -17,7 +17,7 @@ import ThrowableDiagnostic, {
   getJSONSourceLocation,
 } from '@parcel/diagnostic';
 import path from 'path';
-import jsonMap, {type Mapping} from 'json-source-map';
+import {parse, type Mapping} from '@mischnic/json-sourcemap';
 import {
   type ProjectPath,
   fromProjectPath,
@@ -345,7 +345,7 @@ export class EntryResolver {
     return {
       ...pkg,
       filePath: pkgFile,
-      map: jsonMap.parse(content.replace(/\t/g, ' ')),
+      map: parse(content, undefined, {tabWidth: 1}),
     };
   }
 }
diff --git a/packages/core/core/src/requests/TargetRequest.js b/packages/core/core/src/requests/TargetRequest.js
@@ -34,7 +34,7 @@ import createParcelConfigRequest, {
 } from './ParcelConfigRequest';
 // $FlowFixMe
 import browserslist from 'browserslist';
-import jsonMap from 'json-source-map';
+import {parse} from '@mischnic/json-sourcemap';
 import invariant from 'assert';
 import nullthrows from 'nullthrows';
 import {
@@ -395,7 +395,7 @@ export class TargetResolver {
       let _pkgFilePath = (pkgFilePath = pkgFile.filePath); // For Flow
       pkgDir = path.dirname(_pkgFilePath);
       pkgContents = await this.fs.readFile(_pkgFilePath, 'utf8');
-      pkgMap = jsonMap.parse(pkgContents.replace(/\t/g, ' '));
+      pkgMap = parse(pkgContents, undefined, {tabWidth: 1});
 
       let pp = toProjectPath(this.options.projectRoot, _pkgFilePath);
       this.api.invalidateOnFileUpdate(pp);

diff --git a/packages/core/core/test/ParcelConfigRequest.test.js b/packages/core/core/test/ParcelConfigRequest.test.js
@@ -813,6 +813,45 @@ describe('ParcelConfigRequest', () => {
       );
     });
 
+    it('should emit a codeframe when an extended parcel config file is not found in JSON5', async () => {
+      let configFilePath = path.join(
+        __dirname,
+        'fixtures',
+        'config-extends-not-found',
+        '.parcelrc-json5',
+      );
+      let code = await DEFAULT_OPTIONS.inputFS.readFile(configFilePath, 'utf8');
+
+      // $FlowFixMe[prop-missing]
+      await assert.rejects(
+        () => parseAndProcessConfig(configFilePath, code, DEFAULT_OPTIONS),
+        {
+          name: 'Error',
+          diagnostics: [
+            {
+              message: 'Cannot find extended parcel config',
+              origin: '@parcel/core',
+              codeFrames: [
+                {
+                  filePath: configFilePath,
+                  language: 'json5',
+                  code,
+                  codeHighlights: [
+                    {
+                      message:
+                        '"./.parclrc-node-modules" does not exist, did you mean "./.parcelrc-node-modules"?',
+                      start: {line: 2, column: 12},
+                      end: {line: 2, column: 36},
+                    },
+                  ],
+                },
+              ],
+            },
+          ],
+        },
+      );
+    });
+
     it('should emit a codeframe when an extended parcel config node module is not found', async () => {
       let configFilePath = path.join(
         __dirname,

diff --git a/packages/core/core/test/fixtures/config-extends-not-found/.parcelrc-json5 b/packages/core/core/test/fixtures/config-extends-not-found/.parcelrc-json5
@@ -0,0 +1,3 @@
+{
+  extends: "./.parclrc-node-modules"
+}
diff --git a/packages/core/diagnostic/package.json b/packages/core/diagnostic/package.json
@@ -24,7 +24,7 @@
     "check-ts": "tsc --noEmit lib/diagnostic.d.ts"
   },
   "dependencies": {
-    "json-source-map": "^0.6.1",
+    "@mischnic/json-sourcemap": "^0.1.0",
     "nullthrows": "^1.1.1"
   }
 }
diff --git a/packages/core/diagnostic/src/diagnostic.js b/packages/core/diagnostic/src/diagnostic.js
@@ -2,7 +2,7 @@
 
 import invariant from 'assert';
 import nullthrows from 'nullthrows';
-import jsonMap, {type Mapping} from 'json-source-map';
+import {parse, type Mapping} from '@mischnic/json-sourcemap';
 
 /** These positions are 1-based (so <code>1</code> is the first line/column) */
 export type DiagnosticHighlightLocation = {|
@@ -215,8 +215,8 @@ export default class ThrowableDiagnostic extends Error {
 }
 
 /**
- * Turns a list of positions in a JSON file with messages into a list of diagnostics.
- * Uses <a href="https://github.com/epoberezkin/json-source-map">epoberezkin/json-source-map</a>.
+ * Turns a list of positions in a JSON5 file with messages into a list of diagnostics.
+ * Uses <a href="https://github.com/mischnic/json-sourcemap">@mischnic/json-sourcemap</a>.
  *
  * @param code the JSON code
  * @param ids A list of JSON keypaths (<code>key: "/some/parent/child"</code>) with corresponding messages, \
@@ -231,9 +231,10 @@ export function generateJSONCodeHighlights(
       |},
   ids: Array<{|key: string, type?: ?'key' | 'value', message?: string|}>,
 ): Array<DiagnosticCodeHighlight> {
-  // json-source-map doesn't support a tabWidth option (yet)
   let map =
-    typeof data == 'string' ? jsonMap.parse(data.replace(/\t/g, ' ')) : data;
+    typeof data == 'string'
+      ? parse(data, undefined, {dialect: 'JSON5', tabWidth: 1})
+      : data;
   return ids.map(({key, type, message}) => {
     let pos = nullthrows(map.pointers[key]);
     return {
@@ -244,7 +245,7 @@ export function generateJSONCodeHighlights(
 }
 
 /**
- * Converts entries in <a href="https://github.com/epoberezkin/json-source-map">epoberezkin/json-source-map</a>'s
+ * Converts entries in <a href="https://github.com/mischnic/json-sourcemap">@mischnic/json-sourcemap</a>'s
  * <code>result.pointers</code> array.
  */
 export function getJSONSourceLocation(
@@ -276,7 +277,7 @@ export function getJSONSourceLocation(
 
 /** Sanitizes object keys before using them as <code>key</code> in generateJSONCodeHighlights */
 export function encodeJSONKeyComponent(component: string): string {
-  return component.replace(/\//g, '~1');
+  return component.replace(/~/g, '~0').replace(/\//g, '~1');
 }
 
 const escapeCharacters = ['\\', '*', '_', '~'];

diff --git a/packages/core/integration-tests/test/babel.js b/packages/core/integration-tests/test/babel.js
@@ -711,4 +711,60 @@ describe('babel', function () {
       },
     ]);
   });
+
+  it('should warn when a JSON5 babel config contains redundant plugins', async function () {
+    let messages = [];
+    let loggerDisposable = Logger.onLog(message => {
+      messages.push(message);
+    });
+    let filePath = path.join(
+      __dirname,
+      '/integration/babel-warn-some-json5/index.js',
+    );
+    await bundle(filePath);
+    loggerDisposable.dispose();
+
+    let babelrcPath = path.resolve(path.dirname(filePath), '.babelrc');
+    assert.deepEqual(messages, [
+      {
+        type: 'log',
+        level: 'warn',
+        diagnostics: [
+          {
+            origin: '@parcel/transformer-babel',
+            message: md`Parcel includes transpilation by default. Babel config __${path.relative(
+              process.cwd(),
+              babelrcPath,
+            )}__ includes the following redundant presets: __@parcel/babel-preset-env__. Removing these may improve build performance.`,
+            codeFrames: [
+              {
+                filePath: babelrcPath,
+                codeHighlights: [
+                  {
+                    message: undefined,
+                    start: {
+                      line: 2,
+                      column: 13,
+                    },
+                    end: {
+                      line: 2,
+                      column: 38,
+                    },
+                  },
+                ],
+              },
+            ],
+            hints: [
+              md`Remove the above presets from __${path.relative(
+                process.cwd(),
+                babelrcPath,
+              )}__`,
+            ],
+            documentationURL:
+              'https://parceljs.org/languages/javascript/#default-presets',
+          },
+        ],
+      },
+    ]);
+  });
 });
diff --git a/packages/core/integration-tests/test/integration/babel-warn-some-json5/.babelrc b/packages/core/integration-tests/test/integration/babel-warn-some-json5/.babelrc
@@ -0,0 +1,4 @@
+{
+  presets: ["@parcel/babel-preset-env"],
+  "plugins": ["../babelrc-custom/babel-plugin-dummy"]
+}
diff --git a/packages/core/integration-tests/test/integration/babel-warn-some-json5/index.js b/packages/core/integration-tests/test/integration/babel-warn-some-json5/index.js
diff --git a/packages/core/integration-tests/test/integration/babel-warn-some-json5/yarn.lock b/packages/core/integration-tests/test/integration/babel-warn-some-json5/yarn.lock
diff --git a/packages/core/utils/src/schema.js b/packages/core/utils/src/schema.js
@@ -4,7 +4,7 @@ import ThrowableDiagnostic, {
   escapeMarkdown,
   encodeJSONKeyComponent,
 } from '@parcel/diagnostic';
-import type {Mapping} from 'json-source-map';
+import type {Mapping} from '@mischnic/json-sourcemap';
 import nullthrows from 'nullthrows';
 // flowlint-next-line untyped-import:off
 import levenshtein from 'fastest-levenshtein';

diff --git a/packages/transformers/babel/src/config.js b/packages/transformers/babel/src/config.js
@@ -394,12 +394,7 @@ async function getCodeHighlights(fs, filePath, redundantPresets) {
     }
 
     if (pointers.length > 0) {
-      try {
-        return generateJSONCodeHighlights(contents, pointers);
-      } catch {
-        // TODO: support code highlights for json5 sources.
-        // Babel supports json5 syntax, but json-source-map does not.
-      }
+      return generateJSONCodeHighlights(contents, pointers);
     }
   }
 

diff --git a/packages/transformers/webextension/package.json b/packages/transformers/webextension/package.json
@@ -19,10 +19,10 @@
     "parcel": "^2.4.1"
   },
   "dependencies": {
+    "@mischnic/json-sourcemap": "^0.1.0",
     "@parcel/diagnostic": "2.4.1",
     "@parcel/plugin": "2.4.1",
     "@parcel/utils": "2.4.1",
-    "content-security-policy-parser": "^0.3.0",
-    "json-source-map": "^0.6.1"
+    "content-security-policy-parser": "^0.3.0"
   }
 }
diff --git a/packages/transformers/webextension/src/WebExtensionTransformer.js b/packages/transformers/webextension/src/WebExtensionTransformer.js
@@ -3,7 +3,7 @@ import type {MutableAsset} from '@parcel/types';
 
 import {Transformer} from '@parcel/plugin';
 import path from 'path';
-import jsm from 'json-source-map';
+import {parse} from '@mischnic/json-sourcemap';
 import parseCSP from 'content-security-policy-parser';
 import {validateSchema} from '@parcel/utils';
 import ThrowableDiagnostic, {
@@ -362,7 +362,7 @@ export default (new Transformer({
       },
     });
     const code = await asset.getCode();
-    const parsed = jsm.parse(code);
+    const parsed = parse(code);
     const data: any = parsed.data;
 
     // Not using a unified schema dramatically improves error messages

diff --git a/packages/transformers/webmanifest/package.json b/packages/transformers/webmanifest/package.json
@@ -19,9 +19,9 @@
     "parcel": "^2.4.1"
   },
   "dependencies": {
+    "@mischnic/json-sourcemap": "^0.1.0",
     "@parcel/diagnostic": "2.4.1",
     "@parcel/plugin": "2.4.1",
-    "@parcel/utils": "2.4.1",
-    "json-source-map": "^0.6.1"
+    "@parcel/utils": "2.4.1"
   }
 }
diff --git a/packages/transformers/webmanifest/src/WebManifestTransformer.js b/packages/transformers/webmanifest/src/WebManifestTransformer.js
@@ -3,7 +3,7 @@
 import type {SchemaEntity} from '@parcel/utils';
 
 import invariant from 'assert';
-import jsm from 'json-source-map';
+import {parse} from '@mischnic/json-sourcemap';
 import {getJSONSourceLocation} from '@parcel/diagnostic';
 import {Transformer} from '@parcel/plugin';
 import {validateSchema} from '@parcel/utils';
@@ -36,7 +36,7 @@ const MANIFEST_SCHEMA: SchemaEntity = {
 export default (new Transformer({
   async transform({asset}) {
     const source = await asset.getCode();
-    const {data, pointers} = jsm.parse(source);
+    const {data, pointers} = parse(source);
 
     validateSchema.diagnostic(
       MANIFEST_SCHEMA,