Skip to content

Commit

Permalink
ref(eslint): Convert all packages to use central eslint config (#4111)
Browse files Browse the repository at this point in the history
Convert nextjs, react, node, and serverless to using the central eslint config. This should make sure all packages use a consistent eslint config. In our central eslint config, I've disabled our no-async-await rule in tests and made jsdoc required for only exported methods/classes. 

For browser we've kept ignorePatterns: ['test/integration/**', 'src/loader.js'],. I think it's fine to not lint the loader, and the integration tests are going to be refactored with the release stability work, so we can keep this for now. 

Note: Ember is not converted as it's special cased (part of the build) - so I kept it separate. We can have a discussion about what we wanna do with ember, but my opinion is that it's fine to keep it a special case.
  • Loading branch information
AbhiPrasad committed Nov 4, 2021
1 parent ac20799 commit dcd549b
Show file tree
Hide file tree
Showing 22 changed files with 48 additions and 153 deletions.
10 changes: 9 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ module.exports = {
parserOptions: {
project: './tsconfig.json',
},
}
},
{
files: ['*.tsx'],
rules: {
// Turn off jsdoc on tsx files until jsdoc is fixed for tsx files
// See: https://github.com/getsentry/sentry-javascript/issues/3871
'jsdoc/require-jsdoc': 'off',
},
},
],
};
48 changes: 2 additions & 46 deletions packages/browser/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,51 +1,7 @@
module.exports = {
root: true,
env: {
es6: true,
browser: true,
},
parserOptions: {
ecmaVersion: 2018,
},
extends: ['@sentry-internal/sdk'],
ignorePatterns: ['build/**', 'dist/**', 'esm/**', 'examples/**', 'scripts/**', 'coverage/**', 'src/loader.js'],
overrides: [
{
files: ['*.ts', '*.tsx', '*.d.ts'],
parserOptions: {
project: './tsconfig.json',
},
},
{
files: ['test/**'],
rules: {
'jsdoc/require-jsdoc': 'off',
'no-console': 'off',
'max-lines': 'off',
'prefer-template': 'off',
'no-unused-expressions': 'off',
'guard-for-in': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
},
},
{
files: ['test/integration/**'],
env: {
mocha: true,
},
rules: {
'no-undef': 'off',
},
},
{
files: ['test/integration/common/**', 'test/integration/suites/**'],
rules: {
'no-unused-vars': 'off',
},
},
],
rules: {
'no-prototype-builtins': 'off',
},
ignorePatterns: ['test/integration/**', 'src/loader.js'],
extends: ['../../.eslintrc.js'],
};
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/trycatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class TryCatch implements Integration {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const proto = global[target] && global[target].prototype;

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, no-prototype-builtins
if (!proto || !proto.hasOwnProperty || !proto.hasOwnProperty('addEventListener')) {
return;
}
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/transports/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export class XHRTransport extends BaseTransport {

request.open('POST', sentryRequest.url);
for (const header in this.options.headers) {
// eslint-disable-next-line no-prototype-builtins
if (this.options.headers.hasOwnProperty(header)) {
request.setRequestHeader(header, this.options.headers[header]);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/browser/test/package/npm-build.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
const fs = require('fs');
const path = require('path');

Expand Down Expand Up @@ -42,7 +43,7 @@ function runTests() {
const bundlePath = path.join(__dirname, 'tmp.js');
const { window } = new JSDOM(``, { runScripts: 'dangerously' });

window.onerror = function() {
window.onerror = function () {
console.error('ERROR thrown in manual test:');
console.error(arguments);
console.error('------------------');
Expand Down
1 change: 1 addition & 0 deletions packages/browser/test/package/test-code.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
const Sentry = require('../../dist/index.js');
const Integrations = require('../../../integrations/dist/dedupe.js');

Expand Down
5 changes: 4 additions & 1 deletion packages/eslint-config-sdk/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ module.exports = {
},
],

// We want to prevent async await usage in our files to prevent uncessary bundle size.
// We want to prevent async await usage in our files to prevent uncessary bundle size. Turned off in tests.
'@sentry-internal/sdk/no-async-await': 'error',

// JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation,
Expand All @@ -151,6 +151,7 @@ module.exports = {
{
require: { ClassDeclaration: true, MethodDefinition: true },
checkConstructors: false,
publicOnly: true,
},
],

Expand All @@ -173,6 +174,8 @@ module.exports = {
'@typescript-eslint/explicit-member-accessibility': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@sentry-internal/sdk/no-async-await': 'off',
},
},
{
Expand Down
27 changes: 4 additions & 23 deletions packages/nextjs/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,14 @@
module.exports = {
root: true,
env: {
es6: true,
browser: true,
node: true,
},
parserOptions: {
ecmaVersion: 2018,
jsx: true,
},
extends: ['@sentry-internal/sdk'],
ignorePatterns: ['build/**', 'dist/**', 'esm/**', 'examples/**', 'scripts/**', 'test/integration/**'],
overrides: [
{
files: ['*.ts', '*.tsx', '*.d.ts'],
parserOptions: {
project: './tsconfig.json',
},
},
{
files: ['test/**'],
rules: {
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
},
},
],
ignorePatterns: ['test/integration/**'],
extends: ['../../.eslintrc.js'],
rules: {
'max-lines': 'off',
'@sentry-internal/sdk/no-async-await': 'off',
'jsdoc/require-jsdoc': 0,
},
}
};
1 change: 1 addition & 0 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import { getSentryRelease } from '@sentry/node';
import { dropUndefinedKeys, logger } from '@sentry/utils';
import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import {
captureException,
configureScope,
Expand Down
3 changes: 3 additions & 0 deletions packages/nextjs/src/utils/metadataBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ export class MetadataBuilder {
this._packageNames = packages;
}

/** JSDoc */
public addSdkMetadata(): void {
this._options._metadata = this._options._metadata || {};
this._options._metadata.sdk = this._getSdkInfo();
}

/** JSDoc */
private _getSdkInfo(): SdkInfo {
return {
name: SDK_NAME,
Expand All @@ -31,6 +33,7 @@ export class MetadataBuilder {
};
}

/** JSDoc */
private _getPackages(): Package[] {
return this._packageNames.map((pkgName: string) => {
return {
Expand Down
32 changes: 2 additions & 30 deletions packages/node/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,9 @@
module.exports = {
root: true,
env: {
node: true,
},
parserOptions: {
ecmaVersion: 2018,
},
extends: ['@sentry-internal/sdk'],
ignorePatterns: ['build/**', 'dist/**', 'esm/**', 'examples/**', 'scripts/**', 'test/manual/**'],
overrides: [
{
files: ['*.ts', '*.tsx', '*.d.ts'],
parserOptions: {
project: './tsconfig.json',
},
},
{
files: ['test/**'],
rules: {
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
},
},
{
files: ['test/**/*.js'],
rules: {
'import/order': 'off',
},
},
],
extends: ['../../.eslintrc.js'],
rules: {
'prefer-rest-params': 'off',
'@typescript-eslint/no-var-requires': 'off',
'@sentry-internal/sdk/no-async-await': 'off',
},
}
};
2 changes: 2 additions & 0 deletions packages/node/src/integrations/console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ function createConsoleWrapper(level: string): (originalConsoleMethod: () => void
sentryLevel = Severity.Log;
}

/* eslint-disable prefer-rest-params */
return function(this: typeof console): void {
if (getCurrentHub().getIntegration(Console)) {
getCurrentHub().addBreadcrumb(
Expand All @@ -66,5 +67,6 @@ function createConsoleWrapper(level: string): (originalConsoleMethod: () => void

originalConsoleMethod.apply(this, arguments);
};
/* eslint-enable prefer-rest-params */
};
}
2 changes: 2 additions & 0 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export class Http implements Integration {

const wrappedHandlerMaker = _createWrappedRequestMethodFactory(this._breadcrumbs, this._tracing);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpModule = require('http');
fill(httpModule, 'get', wrappedHandlerMaker);
fill(httpModule, 'request', wrappedHandlerMaker);
Expand All @@ -64,6 +65,7 @@ export class Http implements Integration {
// If we do, we'd get double breadcrumbs and double spans for `https` calls.
// It has been changed in Node 9, so for all versions equal and above, we patch `https` separately.
if (NODE_VERSION.major && NODE_VERSION.major > 8) {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const httpsModule = require('https');
fill(httpsModule, 'get', wrappedHandlerMaker);
fill(httpsModule, 'request', wrappedHandlerMaker);
Expand Down
25 changes: 3 additions & 22 deletions packages/react/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,18 @@
module.exports = {
root: true,
env: {
es6: true,
browser: true,
},
parserOptions: {
ecmaVersion: 2018,
jsx: true,
},
extends: ['@sentry-internal/sdk', 'plugin:react/recommended', 'plugin:react-hooks/recommended'],
ignorePatterns: ['build/**', 'dist/**', 'esm/**', 'examples/**', 'scripts/**'],
extends: ['../../.eslintrc.js', 'plugin:react/recommended', 'plugin:react-hooks/recommended'],
overrides: [
{
files: ['*.ts', '*.tsx', '*.d.ts'],
parserOptions: {
project: './tsconfig.json',
},
},
{
files: ['*.tsx'],
rules: {
'jsdoc/require-jsdoc': 'off',
},
},
{
files: ['test/**'],
rules: {
'@typescript-eslint/no-explicit-any': 'off',
// Prop types validation is not useful in test environments
'react/prop-types': 'off',
},
},
],
rules: {
'react/prop-types': 'off',
'@typescript-eslint/no-unsafe-member-access': 'off',
},
};
2 changes: 2 additions & 0 deletions packages/react/src/errorboundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ function withErrorBoundary<P extends Record<string, any>>(
WrappedComponent: React.ComponentType<P>,
errorBoundaryOptions: ErrorBoundaryProps,
): React.FC<P> {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const componentDisplayName = WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT;

const Wrapped: React.FC<P> = (props: P) => (
Expand All @@ -165,6 +166,7 @@ function withErrorBoundary<P extends Record<string, any>>(
</ErrorBoundary>
);

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Wrapped.displayName = `errorBoundary(${componentDisplayName})`;

// Copy over static methods from Wrapped component to Profiler HOC
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/profiler.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/no-explicit-any */
import { getCurrentHub, Hub } from '@sentry/browser';
import { Integration, IntegrationClass, Span, Transaction } from '@sentry/types';
Expand Down
4 changes: 2 additions & 2 deletions packages/react/src/reactrouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ function computeRootMatch(pathname: string): Match {
return { path: '/', url: '/', params: {}, isExact: pathname === '/' };
}

/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access */
export function withSentryRouting<P extends Record<string, any>, R extends React.ComponentType<P>>(Route: R): R {
const componentDisplayName = (Route as any).displayName || (Route as any).name;

Expand All @@ -170,4 +170,4 @@ export function withSentryRouting<P extends Record<string, any>, R extends React
// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/13dc4235c069e25fe7ee16e11f529d909f9f3ff8/types/react-router/index.d.ts#L154-L164
return WrappedRoute;
}
/* eslint-enable @typescript-eslint/no-explicit-any */
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access */
28 changes: 2 additions & 26 deletions packages/serverless/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,9 @@
module.exports = {
root: true,
env: {
es6: true,
node: true,
},
parserOptions: {
ecmaVersion: 2018,
},
extends: ['@sentry-internal/sdk'],
ignorePatterns: ['dist/**', 'esm/**'],
overrides: [
{
files: ['*.ts', '*.d.ts'],
parserOptions: {
project: './tsconfig.json',
},
},
{
files: ['test/**'],
rules: {
'no-empty': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
},
},
],
extends: ['../../.eslintrc.js'],
rules: {
'@typescript-eslint/no-var-requires': 'off',
'@sentry-internal/sdk/no-async-await': 'off',
},
}
};
1 change: 1 addition & 0 deletions packages/serverless/src/awsservices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export class AWSServices implements Integration {
*/
public setupOnce(): void {
try {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const awsModule = require('aws-sdk/global') as typeof AWS;
fill(awsModule.Service.prototype, 'makeRequest', wrapMakeRequest);
} catch (e) {
Expand Down
Loading

0 comments on commit dcd549b

Please sign in to comment.