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


-  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

+      if (prop in this && typeof this[prop] === 'function') {
        // 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

 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

  // 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

  // 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

 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


 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

 var fdeb = new u8([
  0, 0, 0, 0, 1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10, 11, 11, 12, 12, 13, 13, /* unused */ 0, 0,
 ]);
 // 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

 */
 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

    }
    // 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

mydea closed this Dec 3, 2024

mydea deleted the enable-biome-linter-rules branch December 3, 2024 09:24

		@@ -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;

Uh oh!

feat: enable biome recommended linter rules #9848

feat: enable biome recommended linter rules #9848

Uh oh!

Conversation

anonrig commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anonrig commented Dec 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anonrig commented Dec 14, 2023 •

edited

Loading

github-actions bot commented Dec 14, 2023 •

edited

Loading