Skip to content

Commit d44a1d9

Browse files
AbhiPrasadAbhiPrasad
authored
feat: Add eslint to sentry-javascript (#2786)
This PR adds an initial implementation of an eslint-config to the mono-repo. It also adds a eslint-plugin-sentry-sdks, a local npm package we use to write our own custom eslint rules, and converts @sentry/browser from eslint to tslint.
1 parent 05a351b commit d44a1d9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+6896
-510
lines changed

‎.eslintignore

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# THIS IS A TEMPORARY FILE
2+
# THIS WILL BE REMOVED AFTER WE FINISH ESLINT UPGRADE
3+
4+
packages/apm/**/*
5+
packages/core/**/*
6+
packages/ember/**/*
7+
packages/gatsby/**/*
8+
packages/hub/**/*
9+
packages/integrations/**/*
10+
packages/minimal/**/*
11+
packages/node/**/*
12+
packages/react/**/*
13+
packages/tracing/**/*
14+
packages/types/**/*
15+
packages/typescript/**/*
16+
packages/utils/**/*

‎.eslintrc.js

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
module.exports = {
2+
root: true,
3+
env: {
4+
node: true,
5+
},
6+
extends: ['prettier', 'eslint:recommended'],
7+
plugins: ['sentry-sdk', 'jsdoc'],
8+
ignorePatterns: ['eslint-plugin-sentry-sdk'],
9+
overrides: [
10+
{
11+
// Configuration for JavaScript files
12+
files: ['*.js'],
13+
rules: {
14+
'no-unused-vars': ['error', { argsIgnorePattern: '^_' }],
15+
},
16+
},
17+
{
18+
// Configuration for typescript files
19+
files: ['*.ts', '*.tsx', '*.d.ts'],
20+
extends: ['plugin:@typescript-eslint/recommended', 'prettier/@typescript-eslint'],
21+
plugins: ['@typescript-eslint'],
22+
parser: '@typescript-eslint/parser',
23+
parserOptions: {
24+
project: './tsconfig.json',
25+
},
26+
rules: {
27+
// Unused variables should be removed unless they are marked with and underscore (ex. _varName).
28+
'@typescript-eslint/no-unused-vars': ['warn', { argsIgnorePattern: '^_' }],
29+
30+
// Make sure that all ts-ignore comments are given a description.
31+
'@typescript-eslint/ban-ts-comment': [
32+
'warn',
33+
{
34+
'ts-ignore': 'allow-with-description',
35+
},
36+
],
37+
38+
// Types usage should be explicit as possible, so we prevent usage of inferrable types.
39+
// This is especially important because we have a public API, so usage needs to be as
40+
// easy to understand as possible.
41+
'@typescript-eslint/no-inferrable-types': 'off',
42+
43+
// Enforce type annotations to maintain consistency. This is especially important as
44+
// we have a public API, so we want changes to be very explicit.
45+
'@typescript-eslint/typedef': ['error', { arrowParameter: false }],
46+
'@typescript-eslint/explicit-function-return-type': 'error',
47+
48+
// Consistent ordering of fields, methods and constructors for classes should be enforced
49+
'@typescript-eslint/member-ordering': 'error',
50+
51+
// Enforce that unbound methods are called within an expected scope. As we frequently pass data between classes
52+
// in SDKs, we should make sure that we are correctly preserving class scope.
53+
'@typescript-eslint/unbound-method': 'error',
54+
55+
// Private and protected members of a class should be prefixed with a leading underscore
56+
'@typescript-eslint/naming-convention': [
57+
'error',
58+
{
59+
selector: 'memberLike',
60+
modifiers: ['private'],
61+
format: ['camelCase'],
62+
leadingUnderscore: 'require',
63+
},
64+
{
65+
selector: 'memberLike',
66+
modifiers: ['protected'],
67+
format: ['camelCase'],
68+
leadingUnderscore: 'require',
69+
},
70+
],
71+
},
72+
},
73+
{
74+
// Configuration for files under src
75+
files: ['src/**/*'],
76+
rules: {
77+
// We want to prevent async await usage in our files to prevent uncessary bundle size.
78+
'sentry-sdk/no-async-await': 'error',
79+
80+
// JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation,
81+
// even if it may seems excessive at times, is important to emphasize. Turned off in tests.
82+
'jsdoc/require-jsdoc': [
83+
'error',
84+
{ require: { ClassDeclaration: true, MethodDefinition: true }, checkConstructors: false },
85+
],
86+
},
87+
},
88+
{
89+
// Configuration for test files
90+
env: {
91+
jest: true,
92+
},
93+
files: ['*.test.ts', '*.test.tsx', '*.test.js', '*.test.jsx'],
94+
rules: {
95+
'max-lines': 'off',
96+
},
97+
},
98+
{
99+
// Configuration for config files like webpack/rollback
100+
files: ['*.config.js'],
101+
parserOptions: {
102+
sourceType: 'module',
103+
ecmaVersion: 2018,
104+
},
105+
},
106+
],
107+
108+
rules: {
109+
// We want to prevent usage of unary operators outside of for loops
110+
'no-plusplus': ['error', { allowForLoopAfterthoughts: true }],
111+
112+
// Disallow usage of console and alert
113+
'no-console': 'error',
114+
'no-alert': 'error',
115+
116+
// Prevent reassignment of function parameters, but still allow object properties to be
117+
// reassigned. We want to enforce immutability when possible, but we shouldn't sacrifice
118+
// too much efficiency
119+
'no-param-reassign': ['error', { props: false }],
120+
121+
// Prefer use of template expression over string literal concatenation
122+
'prefer-template': 'error',
123+
124+
// Limit maximum file size to reduce complexity. Turned off in tests.
125+
'max-lines': 'error',
126+
},
127+
};

‎.vscode/extensions.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
// See http://go.microsoft.com/fwlink/?LinkId=827846
33
// for the documentation about the extensions.json format
4-
"recommendations": ["esbenp.prettier-vscode", "ms-vscode.vscode-typescript-tslint-plugin"]
4+
"recommendations": ["esbenp.prettier-vscode", "ms-vscode.vscode-typescript-tslint-plugin", "dbaeumer.vscode-eslint"]
55
}

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
66
- [ember] feat: Add `@sentry/ember` (#2739)
77
- [apm/tracing] fix: Mark side effects for tracing hub extensions (#2788)
8+
- [browser] ref: Use stronger function return typings (#2786)
89

910
## 5.20.1
1011

‎dangerfile.ts

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,68 @@ import { promisify } from 'util';
44
import { resolve } from 'path';
55
import tslint from 'danger-plugin-tslint';
66
import { prettyResults } from 'danger-plugin-tslint/dist/prettyResults';
7+
import { CLIEngine } from 'eslint';
78

8-
const packages = ['apm', 'browser', 'core', 'hub', 'integrations', 'minimal', 'node', 'types', 'utils'];
9+
const PACKAGES = ['apm', 'core', 'hub', 'integrations', 'minimal', 'node', 'types', 'utils'];
10+
const EXTENSIONS = ['.js', '.jsx', '.ts', '.tsx'];
911

10-
export default async () => {
12+
/**
13+
* Eslint your code with Danger
14+
* Based on fork from: https://github.com/appcelerator/danger-plugin-eslint
15+
*/
16+
async function eslint(): Promise<void[]> {
17+
const allFiles = danger.git.created_files.concat(danger.git.modified_files);
18+
const cli = new CLIEngine({});
19+
// let eslint filter down to non-ignored, matching the extensions expected
20+
const filesToLint = allFiles.filter(f => !cli.isPathIgnored(f) && EXTENSIONS.some(ext => f.endsWith(ext)));
21+
return Promise.all(filesToLint.map(f => lintFile(cli, f)));
22+
}
23+
24+
/** JSDoc */
25+
async function lintFile(linter: CLIEngine, path: string): Promise<void> {
26+
const contents = await danger.github.utils.fileContents(path);
27+
const report = linter.executeOnText(contents, path);
28+
29+
if (report.results.length !== 0) {
30+
report.results[0].messages.map(msg => {
31+
if (msg.fatal) {
32+
fail(`Fatal error linting ${path} with eslint.`);
33+
return;
34+
}
35+
36+
const noop = (): void => undefined;
37+
const fn = { 0: noop, 1: warn, 2: fail }[msg.severity];
38+
39+
fn(`${path} line ${msg.line} – ${msg.message} (${msg.ruleId})`, path, msg.line);
40+
});
41+
}
42+
}
43+
44+
export default async (): Promise<void> => {
1145
if (!danger.github) {
1246
return;
1347
}
1448

1549
schedule(async () => {
16-
const tsLintResult = (await Promise.all(
17-
packages.map(packageName => {
18-
return new Promise<string>(res => {
19-
tslint({
20-
lintResultsJsonPath: resolve(__dirname, 'packages', packageName, 'lint-results.json'),
21-
handleResults: results => {
22-
if (results.length > 0) {
23-
const formattedResults = prettyResults(results);
24-
res(`TSLint failed: **@sentry/${packageName}**\n\n${formattedResults}`);
25-
} else {
26-
res('');
27-
}
28-
},
50+
const tsLintResult = (
51+
await Promise.all(
52+
PACKAGES.map(packageName => {
53+
return new Promise<string>(res => {
54+
tslint({
55+
lintResultsJsonPath: resolve(__dirname, 'packages', packageName, 'lint-results.json'),
56+
handleResults: results => {
57+
if (results.length > 0) {
58+
const formattedResults = prettyResults(results);
59+
res(`TSLint failed: **@sentry/${packageName}**\n\n${formattedResults}`);
60+
} else {
61+
res('');
62+
}
63+
},
64+
});
2965
});
30-
});
31-
}),
32-
)).filter(str => str.length);
66+
}),
67+
)
68+
).filter(str => str.length);
3369
if (tsLintResult.length) {
3470
tsLintResult.forEach(tsLintFail => {
3571
fail(`${tsLintFail}`);
@@ -39,6 +75,8 @@ export default async () => {
3975
}
4076
});
4177

78+
await eslint();
79+
4280
const hasChangelog = danger.git.modified_files.indexOf('CHANGELOG.md') !== -1;
4381
const isTrivial = (danger.github.pr.body + danger.github.pr.title).includes('#trivial');
4482

‎eslint-plugin-sentry-sdk/index.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// This is a temporary file. It will be removed when we migrate
2+
// the eslint configs to another repo.
3+
4+
'use strict';
5+
6+
/**
7+
* Rule to disallow usage of async await
8+
* @author Abhijeet Prasad
9+
*/
10+
const noAsyncAwait = {
11+
meta: {
12+
type: 'problem',
13+
docs: {
14+
description: 'Disallow usage of async await',
15+
category: 'Best Practices',
16+
recommended: true,
17+
},
18+
fixable: null,
19+
schema: [],
20+
},
21+
create: function(context) {
22+
return {
23+
FunctionDeclaration(node) {
24+
if (node.async) {
25+
context.report({
26+
node,
27+
message:
28+
'Using async-await can add a lot to bundle size. Please do not use it outside of tests, use Promises instead',
29+
});
30+
}
31+
},
32+
33+
ArrowFunctionExpression(node) {
34+
if (node.async) {
35+
context.report({
36+
node,
37+
message:
38+
'Using async-await can add a lot to bundle size. Please do not use it outside of tests, use Promises instead',
39+
});
40+
}
41+
},
42+
};
43+
},
44+
};
45+
46+
module.exports = {
47+
rules: {
48+
'no-async-await': noAsyncAwait,
49+
},
50+
};

‎eslint-plugin-sentry-sdk/package.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"name": "eslint-plugin-sentry-sdk",
3+
"version": "0.0.1",
4+
"main": "index.js",
5+
"devDependencies": {
6+
"eslint": "^7.5.0"
7+
},
8+
"engines": {
9+
"node": ">=0.10.0"
10+
}
11+
}

‎package.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,17 @@
4343
"@types/mocha": "^5.2.0",
4444
"@types/node": "^11.13.7",
4545
"@types/sinon": "^7.0.11",
46+
"@typescript-eslint/eslint-plugin": "^3.7.1",
47+
"@typescript-eslint/parser": "^3.7.1",
4648
"chai": "^4.1.2",
4749
"codecov": "^3.6.5",
4850
"danger": "^7.1.3",
51+
"danger-plugin-eslint": "^0.1.0",
4952
"danger-plugin-tslint": "^2.0.0",
53+
"eslint": "^7.5.0",
54+
"eslint-config-prettier": "^6.11.0",
55+
"eslint-plugin-jsdoc": "^30.0.3",
56+
"eslint-plugin-sentry-sdk": "file:./eslint-plugin-sentry-sdk",
5057
"jest": "^24.7.1",
5158
"karma-browserstack-launcher": "^1.5.1",
5259
"karma-firefox-launcher": "^1.1.0",

‎packages/browser/.eslintrc.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
module.exports = {
2+
root: true,
3+
env: {
4+
es6: true,
5+
browser: true,
6+
},
7+
parserOptions: {
8+
ecmaVersion: 2018,
9+
},
10+
extends: ['../../.eslintrc.js'],
11+
ignorePatterns: ['build/**/*', 'dist/**/*', 'esm/**/*', 'examples/**/*', 'scripts/**/*', 'src/loader.js'],
12+
overrides: [
13+
{
14+
files: ['*.ts', '*.tsx', '*.d.ts'],
15+
parserOptions: {
16+
project: './tsconfig.json',
17+
},
18+
},
19+
{
20+
files: ['test/**/*'],
21+
rules: {
22+
'jsdoc/require-jsdoc': 'off',
23+
'no-console': 'off',
24+
'max-lines': 'off',
25+
'prefer-template': 'off',
26+
},
27+
},
28+
{
29+
files: ['test/integration/**/*'],
30+
env: {
31+
mocha: true,
32+
},
33+
rules: {
34+
'no-undef': 'off',
35+
},
36+
},
37+
{
38+
files: ['test/integration/common/**/*', 'test/integration/suites/**/*'],
39+
rules: {
40+
'no-unused-vars': 'off',
41+
},
42+
},
43+
],
44+
rules: {
45+
'no-prototype-builtins': 'off',
46+
},
47+
};

0 commit comments

Comments
 (0)