feat: enable biome recommended linter rules #9848

anonrig · 2023-12-14T16:16:22Z

Please review with caution and make sure you go through all lines.

Found several bugs on Biome:

I think we should enable some of these rules after this PR is merged:

{
    "rules": {
      "recommended": true,
      "a11y": {
        "all": false
      },
      "complexity": {
        "noForEach": "off",
        "noUselessConstructor": "off",
        "noStaticOnlyClass": "off",
        "useOptionalChain": "off",
        "useLiteralKeys": "off"
      },
      "correctness": {
        "noEmptyPattern": "off",
        "noInnerDeclarations": "off",
        "noVoidTypeReturn": "off",
        "noEmptyCharacterClassInRegex": "off"
      },
      "performance": {
        "noDelete": "off"
      },
      "security": {
        "all": true
      },
      "style": {
        "noArguments": "off",
        "noCommaOperator": "off",
        "noNonNullAssertion": "off",
        "noUselessElse": "off",
        "useTemplate": "off",
        "noVar": "off",
        "useSingleVarDeclarator": "off",
        "useDefaultParameterLast": "off",
        "useExponentiationOperator": "off"
      },
      "suspicious": {
        "noDebugger": "off",
        "noDoubleEquals": "off",
        "noExplicitAny": "off",
        "noRedeclare": "off",
        "noAssignInExpressions": "off",
        "noConfusingVoidType": "off",
        "noPrototypeBuiltins": "off",
        "noShadowRestrictedNames": "off",
        "noControlCharactersInRegex": "off"
      }
    }
}

packages/angular-ivy/scripts/prepack.ts

packages/browser-integration-tests/suites/replay/bufferMode/test.ts

github-actions · 2023-12-14T17:54:55Z

size-limit report 📦

Path	Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped)	75.29 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)	66.63 KB (+0.01% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped)	60.21 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)	31.29 KB (-0.01% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped)	29.88 KB (-0.07% 🔽)
@sentry/browser - Webpack (gzipped)	21.54 KB (-0.1% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped)	72.57 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)	64.3 KB (-0.03% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)	30.56 KB (-0.08% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped)	22.6 KB (-0.05% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)	202.34 KB (-0.08% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)	92.51 KB (-0.16% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)	67.46 KB (-0.25% 🔽)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)	33.47 KB (-0.02% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)	66.97 KB (-0.02% 🔽)
@sentry/react - Webpack (gzipped)	21.58 KB (-0.1% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)	83.71 KB (-0.03% 🔽)
@sentry/nextjs Client - Webpack (gzipped)	48.41 KB (-0.01% 🔽)
@sentry-internal/feedback - Webpack (gzipped)	16.19 KB (-0.04% 🔽)

mydea · 2023-12-15T10:45:29Z

packages/browser/src/integrations/globalhandlers.ts

@@ -236,8 +236,8 @@ function _enhanceEventWithInitialFrame(event: Event, url: any, line: any, column
 // event.exception.values[0].stacktrace.frames
 const ev0sf = (ev0s.frames = ev0s.frames || []);

- const colno = isNaN(parseInt(column, 10)) ? undefined : column;
- const lineno = isNaN(parseInt(line, 10)) ? undefined : line;
+ const colno = Number.isNaN(parseInt(column, 10)) ? undefined : column;


Number.isNaN is not supported by IE11 😬 so we need to remove that rule and revert these, sadly, at least until V8!

mydea · 2023-12-15T10:46:47Z

packages/browser/src/integrations/trycatch.ts

 // eslint-disable-next-line @typescript-eslint/no-explicit-any
- fill(xhr, prop, function (original: WrappedFunction): () => any {
+ fill(this, prop, (original: WrappedFunction): (() => any) => {


I am not 100% sure if it is safe to replace these with anonymous functions - we should just double check, I remember seeing some comments somewhere that in some cases this may be tricky.

mydea · 2023-12-15T10:48:01Z

packages/browser/src/loader.js

@@ -1,5 +1,5 @@
 // biome-ignore format: Disabled due to trailing comma not working in IE10/11


We need to exclude this file, as this is specifically copy-pasted from the sentry repo (where the loader actually lives), and should be exactly the same as there!

mydea · 2023-12-15T10:51:24Z

packages/browser/src/profiling/utils.ts

@@ -416,7 +416,7 @@ export function applyDebugMetadata(resource_paths: ReadonlyArray<string>): Debug
 */
 export function isValidSampleRate(rate: unknown): boolean {
 // we need to check NaN explicitly because it's of type 'number' and therefore wouldn't get caught by this typecheck
- if ((typeof rate !== 'number' && typeof rate !== 'boolean') || (typeof rate === 'number' && isNaN(rate))) {
+ if ((typeof rate !== 'number' && typeof rate !== 'boolean') || (typeof rate === 'number' && Number.isNaN(rate))) {


mydea · 2023-12-15T10:51:50Z

packages/browser/src/transports/offline.ts

@@ -107,7 +107,7 @@ function createIndexedDbStore(options: BrowserOfflineTransportOptions): OfflineS

 // Lazily create the store only when it's needed
 function getStore(): Store {
- if (store == undefined) {
+ if (store === undefined) {


l: can we just make this:

Suggested change

if (store === undefined) {

if (!store) {

mydea · 2023-12-15T10:54:09Z

packages/core/src/tracing/sampling.ts

@@ -103,7 +103,7 @@ export function sampleTransaction<T extends Transaction>(
 function isValidSampleRate(rate: unknown): boolean {
 // we need to check NaN explicitly because it's of type 'number' and therefore wouldn't get caught by this typecheck
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
- if (isNaN(rate) || !(typeof rate === 'number' || typeof rate === 'boolean')) {
+ if (Number.isNaN(rate) || !(typeof rate === 'number' || typeof rate === 'boolean')) {


mydea · 2023-12-15T10:55:55Z

packages/e2e-tests/test-applications/nextjs-app-dir/assert-build.ts

@@ -19,5 +19,3 @@ assert.match(buildStdout, /● \/client-component\/parameter\/\[parameter\]/);
 assert.match(buildStdout, /λ \/server-component/);
 assert.match(buildStdout, /λ \/server-component\/parameter\/\[\.\.\.parameters\]/);
 assert.match(buildStdout, /λ \/server-component\/parameter\/\[parameter\]/);
-
-export {};


I think this is needed for build reasons 🤔

mydea · 2023-12-15T10:57:18Z

packages/gatsby/gatsby-browser.js

@@ -39,7 +39,7 @@ function isSentryInitialized() {
 }

 function areSentryOptionsDefined(params) {
- if (params == undefined) return false;
+ if (params === undefined) return false;


Suggested change

if (params === undefined) return false;

if (!params) return false;

mydea · 2023-12-15T11:01:06Z

packages/replay-worker/examples/worker.js

@@ -17,7 +17,7 @@ var fdeb = new u8([
 // code length index map


This is a built file, we need to exclude this.

mydea · 2023-12-15T11:03:03Z

packages/tracing-internal/src/browser/metrics/utils.ts

@@ -5,7 +5,7 @@ import type { Span, SpanContext } from '@sentry/types';
 * Checks if a given value is a valid measurement value.
 */
 export function isMeasurementValue(value: unknown): value is number {
- return typeof value === 'number' && isFinite(value);
+ return typeof value === 'number' && Number.isFinite(value);


mydea · 2023-12-15T11:03:17Z

packages/tracing-internal/src/browser/request.ts

@@ -175,7 +175,7 @@ export function extractNetworkProtocol(nextHopProtocol: string): { name: string;
 break;
 }
 // h2, h3 etc.
- if (!isNaN(Number(char))) {
+ if (!Number.isNaN(Number(char))) {


mydea · 2023-12-15T11:05:50Z

packages/utils/src/aggregate-errors.ts

@@ -9,7 +9,7 @@ import { truncate } from './string';
 export function applyAggregateErrorsToEvent(
 exceptionFromErrorImplementation: (stackParser: StackParser, ex: Error) => Exception,
 parser: StackParser,
- maxValueLimit: number = 250,
+ maxValueLimit: number | undefined = 250,


why this change? 🤔

mydea · 2023-12-15T11:06:28Z

packages/utils/src/dsn.ts

@@ -107,7 +107,7 @@ function validateDsn(dsn: DsnComponents): boolean {
 return false;
 }

- if (port && isNaN(parseInt(port, 10))) {
+ if (port && Number.isNaN(parseInt(port, 10))) {


mydea · 2023-12-15T11:10:27Z

packages/utils/src/is.ts

- * @param wat A value to be checked.
- * @returns A boolean representing the result.
- */
-export function isNaN(wat: unknown): boolean {


I don't think we can get rid of this, as this is exported and thus public API.

mydea · 2023-12-15T11:10:53Z

packages/utils/src/misc.ts

- major: isNaN(major) ? undefined : major,
- minor: isNaN(minor) ? undefined : minor,
- patch: isNaN(patch) ? undefined : patch,
+ major: Number.isNaN(major) ? undefined : major,


mydea · 2023-12-15T11:11:51Z

packages/utils/src/normalize.ts

 memo: MemoFunc = memoBuilder(),
 ): Primitive | ObjOrArray<unknown> {
 const [memoize, unmemoize] = memo;

 // Get the simple cases out of the way first
 if (
 value == null || // this matches null and undefined -> eqeq not eqeqeq
- (['number', 'boolean', 'string'].includes(typeof value) && !isNaN(value))
+ (['number', 'boolean', 'string'].includes(typeof value) && !Number.isNaN(value))


OK, I wont repeat this anymore, but for everything that's not node only we can't use Number.isNaN 😅

anonrig · 2023-12-19T01:13:28Z

I'll keep this PR open, and revisit it when we start working on v8. @mydea

anonrig force-pushed the enable-biome-linter-rules branch 3 times, most recently from 204c92d to 3e5fb88 Compare December 14, 2023 16:24

AbhiPrasad reviewed Dec 14, 2023

View reviewed changes

packages/angular-ivy/scripts/prepack.ts Outdated Show resolved Hide resolved

packages/browser-integration-tests/suites/replay/bufferMode/test.ts Outdated Show resolved Hide resolved

anonrig force-pushed the enable-biome-linter-rules branch from 3e5fb88 to 44f5bda Compare December 14, 2023 17:46

anonrig added ci-codecov-ai-review AI Review by Codecov and removed ci-codecov-ai-review AI Review by Codecov labels Dec 14, 2023

anonrig force-pushed the enable-biome-linter-rules branch 5 times, most recently from 48f2fc5 to 0f9a3ca Compare December 14, 2023 21:33

feat: enable biome recommended linter rules

d4d1ab2

anonrig force-pushed the enable-biome-linter-rules branch from 0f9a3ca to d4d1ab2 Compare December 14, 2023 22:36

mydea reviewed Dec 15, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable biome recommended linter rules #9848

feat: enable biome recommended linter rules #9848

anonrig commented Dec 14, 2023 •

edited

Loading

github-actions bot commented Dec 14, 2023 •

edited

Loading

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

mydea Dec 15, 2023

anonrig commented Dec 19, 2023

		@@ -1,5 +1,5 @@
		// biome-ignore format: Disabled due to trailing comma not working in IE10/11

	if (params === undefined) return false;
	if (!params) return false;

		@@ -17,7 +17,7 @@ var fdeb = new u8([
		// code length index map

feat: enable biome recommended linter rules #9848

Are you sure you want to change the base?

feat: enable biome recommended linter rules #9848

Conversation

anonrig commented Dec 14, 2023 • edited Loading

github-actions bot commented Dec 14, 2023 • edited Loading

size-limit report 📦

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anonrig commented Dec 19, 2023

anonrig commented Dec 14, 2023 •

edited

Loading

github-actions bot commented Dec 14, 2023 •

edited

Loading