Skip to content

ref(eslint): Convert all packages to use central eslint config #4111

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

Merged
merged 9 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)) {
Comment on lines +59 to 60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to just fix this rather than ignore it?

Suggested change
// eslint-disable-next-line no-prototype-builtins
if (this.options.headers.hasOwnProperty(header)) {
if (Object.prototype.hasOwnProperty.call(this, header)) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanted to make the code change as minimal as possible. I'll come back and address these low hanging fruits in separate PRs.

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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious - why turn no-empty-function off?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel like it's not valuable for tests, where you might want to quickly add empty functions if you are stubbing out a class/object.

'@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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this if this is the default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is error (turned on). We turn it off for tests.

'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 */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not letting me do this as a suggestion (I guess because it spans a part which it hides by default), but since it's an easy fix IMHO we should just change arguments to ...args and not ignore the rule.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will circle back to this in a PR afterwards to not increase the scope of this PR

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