ref(nextjs): Always use tsx parser in experimental function-wrapping loader (#5563)

The changes the experimental data-fetching-function-wrapping loader to always use the tsx parser, as it's capable of parsing jsx as well. This in turn allows us to simplify the `makeAST` function.

Author: lobsterkatie

packages/nextjs/src/config/loaders/ast.ts

@@ -2,7 +2,7 @@
 import * as jscsTypes from 'jscodeshift';
 import { default as jscodeshiftDefault } from 'jscodeshift';
 
-import { makeParser } from './parsers';
+import { parser } from './parser';
 
 // In `jscodeshift`, the exports look like this:
 //
@@ -64,12 +64,10 @@ type VariableDeclarationNode = jscsTypes.VariableDeclaration;
  * Create an AST based on the given code.
  *
  * @param code The code to convert to an AST.
- * @param isTS Flag indicating what parser to use.
- * @throws Parsing error if the code is unparsable
+ * @throws Throws parsing error if the code is unparsable
  * @returns The AST
  */
-export function makeAST(code: string, isTS: boolean): AST {
-  const parser = isTS ? makeParser('tsx') : makeParser('jsx');
+export function makeAST(code: string): AST {
   // If this errors, it will be caught in the calling function, where we know more information and can construct a
   // better warning message
   return jscs(code, { parser });

packages/nextjs/src/config/loaders/dataFetchersLoader.ts

@@ -52,11 +52,10 @@ type LoaderOptions = {
  */
 function wrapFunctions(userCode: string, templateCode: string, filepath: string): string[] {
   let userAST: AST, templateAST: AST;
-  const isTS = new RegExp('\\.tsx?$').test(filepath);
 
   try {
-    userAST = makeAST(userCode, isTS);
-    templateAST = makeAST(templateCode, false);
+    userAST = makeAST(userCode);
+    templateAST = makeAST(templateCode);
   } catch (err) {
     logger.warn(`Couldn't add Sentry to ${filepath} because there was a parsing error: ${err}`);
     // Replace the template code with an empty string, so in the end the user code is untouched
@@ -124,8 +123,7 @@ export default function wrapDataFetchersLoader(this: LoaderThis<LoaderOptions>,
   // we know we're in a proxied file and do not need to proxy again.
 
   if (!this.resourceQuery.includes('sentry-proxy-loader')) {
-    const ast = makeAST(userCode, true); // is there a reason to ever parse without typescript?
-
+    const ast = makeAST(userCode);
     const exportedIdentifiers = getExportIdentifierNames(ast);
 
     let outputFileContent = '';

packages/nextjs/src/config/loaders/parsers.ts renamed to packages/nextjs/src/config/loaders/parser.ts

@@ -13,7 +13,7 @@ type Parser = {
   parse: (code: string) => babel.ParseResult<File>;
 };
 
-const jsxOptions: babel.ParserOptions = {
+const options: babel.ParserOptions = {
   // Nextjs supports dynamic import, so this seems like a good idea
   allowImportExportEverywhere: true,
   // We're only supporting wrapping in ESM pages
@@ -40,25 +40,10 @@ const jsxOptions: babel.ParserOptions = {
     ['pipelineOperator', { proposal: 'hack', topicToken: '^' }],
     'regexpUnicodeSets',
     'throwExpressions',
+    'typescript',
   ] as babel.ParserPlugin[],
 };
 
-const tsxOptions = {
-  ...jsxOptions,
-  // Because `jsxOptions` is typed as a `ParserOptions` object, TS doesn't discount the possibility of its `plugins`
-  // property being undefined, even though it is, in fact, very clearly defined - hence the empty array.
-  plugins: [...(jsxOptions.plugins || []), 'typescript'] as babel.ParserPlugin[],
+export const parser: Parser = {
+  parse: code => babel.parse(code, options),
 };
-
-/**
- * Create either a jsx or tsx parser to be used by `jscodeshift`.
- *
- * @param type Either 'jsx' or 'tsx'
- * @returns An object with the appropriate `parse` method.
- */
-export function makeParser(type: 'jsx' | 'tsx'): Parser {
-  const options = type === 'jsx' ? jsxOptions : tsxOptions;
-  return {
-    parse: code => babel.parse(code, options),
-  };
-}

packages/nextjs/test/config/ast.test.ts

@@ -35,7 +35,7 @@ test.each([
   ['export { default } from "module-name";', true],
   ['export { default, name1 } from "module-name";', true],
 ])('hasDefaultExport(%s) should return %p', (program, expectedResult) => {
-  const ast = makeAST(program, true);
+  const ast = makeAST(program);
   expect(hasDefaultExport(ast)).toBe(expectedResult);
 });
 
@@ -95,6 +95,6 @@ test.each([
   ['export { default } from "module-name";', []],
   ['export { default, name1 } from "module-name";', ['name1']],
 ])('getExportIdentifiers(%s) should return %p', (program, expectedIdentifiers) => {
-  const ast = makeAST(program, true);
+  const ast = makeAST(program);
   expect(getExportIdentifierNames(ast)).toStrictEqual(expectedIdentifiers);
 });