ref(build): Remove constToVarPlugin (#5970)

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>

packages/browser/src/integrations/trycatch.ts

@@ -183,9 +183,9 @@ function _wrapXHR(originalSend: () => void): () => void {
 /** JSDoc */
 function _wrapEventTarget(target: string): void {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  const global = WINDOW as { [key: string]: any };
+  const globalObject = WINDOW as { [key: string]: any };
   // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-  const proto = global[target] && global[target].prototype;
+  const proto = globalObject[target] && globalObject[target].prototype;
 
   // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, no-prototype-builtins
   if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) {

packages/nextjs/src/performance/client.ts

@@ -11,7 +11,7 @@ import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
 import { default as Router } from 'next/router';
 import type { ParsedUrlQuery } from 'querystring';
 
-const global = WINDOW as typeof WINDOW & {
+const globalObject = WINDOW as typeof WINDOW & {
   __BUILD_MANIFEST?: {
     sortedPages?: string[];
   };
@@ -57,7 +57,7 @@ function extractNextDataTagInformation(): NextDataTagInfo {
   let nextData: SentryEnhancedNextData | undefined;
   // Let's be on the safe side and actually check first if there is really a __NEXT_DATA__ script tag on the page.
   // Theoretically this should always be the case though.
-  const nextDataTag = global.document.getElementById('__NEXT_DATA__');
+  const nextDataTag = globalObject.document.getElementById('__NEXT_DATA__');
   if (nextDataTag && nextDataTag.innerHTML) {
     try {
       nextData = JSON.parse(nextDataTag.innerHTML);
@@ -122,7 +122,7 @@ export function nextRouterInstrumentation(
   startTransactionOnLocationChange: boolean = true,
 ): void {
   const { route, traceParentData, baggage, params } = extractNextDataTagInformation();
-  prevLocationName = route || global.location.pathname;
+  prevLocationName = route || globalObject.location.pathname;
 
   if (startTransactionOnPageLoad) {
     const source = route ? 'route' : 'url';
@@ -197,7 +197,7 @@ export function nextRouterInstrumentation(
 }
 
 function getNextRouteFromPathname(pathname: string): string | undefined {
-  const pageRoutes = (global.__BUILD_MANIFEST || {}).sortedPages;
+  const pageRoutes = (globalObject.__BUILD_MANIFEST || {}).sortedPages;
 
   // Page route should in 99.999% of the cases be defined by now but just to be sure we make a check here
   if (!pageRoutes) {

rollup/npmHelpers.js

@@ -8,7 +8,6 @@ import * as path from 'path';
 import deepMerge from 'deepmerge';
 
 import {
-  makeConstToVarPlugin,
   makeExtractPolyfillsPlugin,
   makeNodeResolvePlugin,
   makeCleanupPlugin,
@@ -30,7 +29,6 @@ export function makeBaseNPMConfig(options = {}) {
   const nodeResolvePlugin = makeNodeResolvePlugin();
   const sucrasePlugin = makeSucrasePlugin();
   const debugBuildStatementReplacePlugin = makeDebugBuildStatementReplacePlugin();
-  const constToVarPlugin = makeConstToVarPlugin();
   const cleanupPlugin = makeCleanupPlugin();
   const extractPolyfillsPlugin = makeExtractPolyfillsPlugin();
 
@@ -81,7 +79,6 @@ export function makeBaseNPMConfig(options = {}) {
       nodeResolvePlugin,
       sucrasePlugin,
       debugBuildStatementReplacePlugin,
-      constToVarPlugin,
       cleanupPlugin,
       extractPolyfillsPlugin,
     ],

rollup/plugins/npmPlugins.js

@@ -22,32 +22,6 @@ export function makeSucrasePlugin() {
   });
 }
 
-/**
- * Create a plugin to switch all instances of `const` to `var`, both to prevent problems when we shadow `global` and
- * because it's fewer characters.
- *
- * Note that the generated plugin runs the replacement both before and after rollup does its code manipulation, to
- * increase the chances that nothing is missed.
- *
- * TODO This is pretty brute-force-y. Perhaps we could switch to using a parser, the way we (will) do for both our jest
- * transformer and the polyfill build script.
- *
- * @returns An instance of the `@rollup/plugin-replace` plugin
- */
-export function makeConstToVarPlugin() {
-  return replace({
-    // TODO `preventAssignment` will default to true in version 5.x of the replace plugin, at which point we can get rid
-    // of this. (It actually makes no difference in this case whether it's true or false, since we never assign to
-    // `const`, but if we don't give it a value, it will spam with warnings.)
-    preventAssignment: true,
-    values: {
-      // Include a space at the end to guarantee we're not accidentally catching the beginning of the words "constant,"
-      // "constantly," etc.
-      'const ': 'var ',
-    },
-  });
-}
-
 /**
  * Create a plugin which can be used to pause the build process at the given hook.
  *

scripts/test.ts

@@ -1,6 +1,5 @@
 import * as childProcess from 'child_process';
 import * as fs from 'fs';
-import * as path from 'path';
 
 const CURRENT_NODE_VERSION = process.version.replace('v', '').split('.')[0];
 
@@ -62,42 +61,6 @@ function installLegacyDeps(legacyDeps: string[] = []): void {
   run(`yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check ${legacyDeps.join(' ')}`);
 }
 
-/**
- * Add a tranformer to our jest config, to do the same `const`-to-`var` replacement as our rollup plugin does.
- *
- * This is needed because Node 8 doesn't like the way we shadow `global` (`const global = getGlobalObject()`). Changing
- * it to a `var` solves this by making it redeclarable.
- *
- */
-function addJestTransformer(): void {
-  // Though newer `ts-jest` versions support transformers written in TS, the legacy version does not.
-  run('yarn tsc --skipLibCheck jest/transformers/constReplacer.ts');
-
-  // Loading the existing Jest config will error out unless the config file has an accompanying types file, so we have
-  // to create that before we can load it.
-  run('yarn tsc --allowJs --skipLibCheck --declaration --emitDeclarationOnly jest/jest.config.js');
-  // eslint-disable-next-line @typescript-eslint/no-var-requires
-  const jestConfig = require('../jest/jest.config.js');
-
-  // Inject the transformer
-  jestConfig.globals['ts-jest'].astTransformers = ['<rootDir>/../../jest/transformers/constReplacer.js'];
-
-  // When we required the jest config file above, all expressions it contained were evaluated. Specifically, the
-  //     `rootDir: process.cwd()`
-  // entry was replaced with
-  //     `rootDir: "<hard-coded string result of running `process.cwd()` in the current process>"`,
-  // Though it's a little brute-force-y, the easiest way to fix this is to just stringify the code and perform the
-  // substitution in reverse.
-  const stringifiedConfig = JSON.stringify(jestConfig, null, 2).replace(
-    `"rootDir": "${process.cwd()}"`,
-    'rootDir: process.cwd()',
-  );
-
-  // Now we just have to convert it back to a module and write it to disk
-  const code = `module.exports = ${stringifiedConfig}`;
-  fs.writeFileSync(path.resolve('jest/jest.config.js'), code);
-}
-
 /**
  * Modify a json file on disk.
  *
@@ -151,8 +114,6 @@ function runWithIgnores(skipPackages: string[] = []): void {
 function runTests(): void {
   if (CURRENT_NODE_VERSION === '8') {
     installLegacyDeps(NODE_8_LEGACY_DEPENDENCIES);
-    // Inject a `const`-to-`var` transformer, in order to stop Node 8 from complaining when we shadow `global`
-    addJestTransformer();
     // TODO Right now, this just skips incompatible tests, but it could be skipping more (hence the aspirational name),
     // and not just in Node 8. See `skipNonNodeTests`'s docstring.
     skipNonNodeTests();