Repository navigation
chore(lint): Enable type-aware Oxlint rules disabled in #1556 - #1578
Merged
Merged
Conversation
Turn the nine type-aware rules back on and fix the violations they report: drop awaits on non-thenables, mark intentional fire-and-forget promises with void, await forgotten assertions, give sorts explicit comparators, stop stringifying unknown values to "[object Object]", and remove redundant type constituents. no-floating-promises allows node:test's test/describe helpers, restrict-template-expressions allows never, and unbound-method stays off in tests, where mock and identity assertions trip it. Fixes #1573 Co-Authored-By: BYK <burak.kaya@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
BYK
requested changes
Oct 22, 2026
| } catch (err) { | ||
| log.debug("Failed to JSON-encode attribute value", err); | ||
| return sanitize(String(value)); | ||
| return sanitize(Object.prototype.toString.call(value)); |
Contributor
Author
There was a problem hiding this comment.
That was a clumsy fix, sorry. The catch branch only runs when JSON.stringify throws (circular refs, BigInt). The old String(value) there tripped no-base-to-string, and swapping in Object.prototype.toString.call() just produced the same [object Object] in a roundabout way. stringifyUnknown() already does exactly this try-JSON-then-fall-back dance, so formatAttrValue() now just does sanitize(stringifyUnknown(value)) after the null check. Fixed in b75e6a1.
- Count slashes with a non-allocating countChar() helper
- Keep RegExp.escape unbound; it is static and never reads `this`
- Let formatAttrValue() defer to stringifyUnknown()
- Rename fetchInputUrl() to extractInputUrl()
- Restore IssueStatusDisplay using `string & {}`
- Allow URL/URLSearchParams in template literals instead of
sprinkling .href/.toString()
- Narrow replay payload entries with a type guard instead of String()
Per review: keep String() in stringifyUnknown's JSON failure fallback so custom toString() output survives, and keep the `unknown | Handler` unions in the test mock types for documentation. Both use targeted lint disables with reasons.
BYK
approved these changes
Oct 22, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turns back on the nine type-aware rules #1556 switched off (
await-thenable,no-base-to-string,no-duplicate-type-constituents,no-floating-promises,no-misused-spread,no-redundant-type-constituents,require-array-sort-compare,restrict-template-expressions,unbound-method) and fixes the ~640 violations they reported.Most fixes are mechanical. These ones are worth a look:
LoggingUIrejection assertions were never awaited, so they could pass without running. Smoke-test headers object-spreadHeadersInit, which silently drops entries from aHeadersinstance; they now go throughHeaders. Fetch wrappers calledString(input)onRequest | URL | string, which gives[object Request]for requests; a new CLIfetchInputUrl()helper reads the URL properly.String()'d unknown values now use the CLI'sstringifyUnknown()or MCP'sformatUnknown(), so objects show up as JSON instead of[object Object].formatUnknownmoved tomcp-core/src/internal/unknown-formatting.tsso internal formatters don't import fromtools/(it's still re-exported fromapi-formatting). Spotlight envelope IDs are UUID objects with their owntoString(), so they get a smallspotlightEnvelopeId()helper.TraceSpanSchemanow uses Zod 4's getter-based recursion instead ofz.lazy(...) as z.ZodType<any>. Thatanywas hiding redundant union members.getSentryConfig.partialfrom the Cloudflare Sentry config. It was the onlyno-misused-spreadhit that needed real changes.Config tweaks instead of blanket disables:
no-floating-promisesusesallowForKnownSafeCallsfornode:test'stest/it/describe/suite.restrict-template-expressionsallowsnever(exhaustive-switch error messages).unbound-methodis off only in test files, where mock and identity assertions likeexpect(obj.method)trip it all the time.Fixes #1573
via BYK.
--
View Junior Session in Sentry