Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(build): Remove constToVarPlugin #5970

Merged
merged 12 commits into from Oct 18, 2022
84 changes: 0 additions & 84 deletions jest/transformers/constReplacer.ts

This file was deleted.

4 changes: 2 additions & 2 deletions packages/browser/src/integrations/trycatch.ts
Expand Up @@ -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')) {
Expand Down
8 changes: 4 additions & 4 deletions packages/nextjs/src/performance/client.ts
Expand Up @@ -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[];
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 0 additions & 3 deletions rollup/npmHelpers.js
Expand Up @@ -8,7 +8,6 @@ import * as path from 'path';
import deepMerge from 'deepmerge';

import {
makeConstToVarPlugin,
makeExtractPolyfillsPlugin,
makeNodeResolvePlugin,
makeCleanupPlugin,
Expand All @@ -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();

Expand Down Expand Up @@ -81,7 +79,6 @@ export function makeBaseNPMConfig(options = {}) {
nodeResolvePlugin,
sucrasePlugin,
debugBuildStatementReplacePlugin,
constToVarPlugin,
cleanupPlugin,
extractPolyfillsPlugin,
],
Expand Down
26 changes: 0 additions & 26 deletions rollup/plugins/npmPlugins.js
Expand Up @@ -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.
*
Expand Down
39 changes: 0 additions & 39 deletions 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];

Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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();
Expand Down