feat(repo): Introduce oxlint #6105
Conversation
🦋 Changeset detectedLatest commit: 82f10bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis update introduces OXLint as a new linting tool across the repository. A root configuration file for OXLint is added, and package scripts are updated to include OXLint commands for individual packages. Inline lint rule suppression comments are inserted in source files to address specific OXLint warnings. The monorepo's task runner configuration is updated to incorporate the new linting workflow. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (9)
packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts (1)
48-50: 🛠️ Refactor suggestionSame unchecked optional-chaining risk as above
The pattern repeats here. Either assert
res?.responseis present or refine_fetch’s typing; don’t just mute the rule.packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (1)
31-34: 🛠️ Refactor suggestionUnchecked
res?.responsemay throw – handle explicitlySee previous comments: the destructuring will blow up when
responseis absent. Failing fast with a clear error (or improving_fetchtyping) is preferable to blanket-disabling the lint.packages/clerk-js/src/core/resources/Organization.ts (5)
124-126: 🛠️ Refactor suggestionSame concern as above – unsafe optional chaining muted instead of handled.
152-154: 🛠️ Refactor suggestionSame pattern – please address globally.
174-176: 🛠️ Refactor suggestionAs above.
197-199: 🛠️ Refactor suggestionAs above.
246-248: 🛠️ Refactor suggestionAs above.
packages/astro/src/astro-components/control/ProtectCSR.astro (1)
52-54: Mirror the refactor for the fallback slot-// oxlint-disable-next-line no-unused-expressions -isUnauthorized ? this.fallbackSlot.removeAttribute('hidden') : this.fallbackSlot.setAttribute('hidden', ''); +this.fallbackSlot?.toggleAttribute('hidden', !isUnauthorized);packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (1)
69-72: Duplicate of the issue above forgetStatementsSame nullable-response risk and lint suppression here; please apply the same fix.
🧹 Nitpick comments (19)
packages/clerk-js/svgTransform.js (1)
1-3: Streamline lint suppression directives
Since the plan is to fully migrate off ESLint, consider replacing the ESLint disable comment with Oxlint’s equivalent forno-var-requiresto keep consistency across the codebase:- // eslint-disable-next-line @typescript-eslint/no-var-requires + // oxlint-disable-next-line @typescript-eslint/no-var-requirespackages/backend/src/runtime/node/crypto.js (1)
1-2: Consistency: Add OXLint disable directive
To align with other modules adopting Oxlint, consider adding an// oxlint-disable-next-line @typescript-eslint/no-require-importsalongside the existing ESLint directive for uniform suppression.packages/astro/src/astro-components/control/ProtectCSR.astro (1)
47-49: UsetoggleAttributeto avoid side-effect ternaries & lint disableTernary expressions used only for side-effects trigger
no-unused-expressions.
A clearer, lint-friendly alternative:-// oxlint-disable-next-line no-unused-expressions -isUnauthorized ? this.defaultSlot.setAttribute('hidden', '') : this.defaultSlot.removeAttribute('hidden'); +this.defaultSlot?.toggleAttribute('hidden', isUnauthorized);packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (1)
90-91: Stale disable comment
// oxlint-disable-next-line no-unsafe-optional-chainingrefers to an optional-chaining rule, but the next line no longer uses?.. Remove the comment to avoid dead-code noise.-// oxlint-disable-next-line no-unsafe-optional-chaining const { data: payments, total_count } = res as unknown as ClerkPaginatedResponse<CommercePaymentJSON>;packages/clerk-js/src/utils/url.ts (2)
247-252: Mixed ESLint/OXLint directives – pick one linter or keep both consistentlyYou added
// oxlint-disable-next-line no-control-regexbut kept the old ESLint directive two lines below.
If ESLint will eventually be removed, drop the ESLint comment; if both linters stay, duplicate directives are fine but noisy. Choose and keep consistent.🧰 Tools
🪛 Biome (1.9.4)
[error] 251-251: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 251-251: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
262-266: Redundant null-byte check – regex already covers it
url.pathname.includes('\0')is functionally equivalent to the/\0/pattern you added todisallowedPatterns, so we now check the same thing twice. Prefer a single source of truth (regex or explicit string check) to avoid drift.-// Check for null bytes in pathname -if (url.pathname.includes('\0')) { - return true; -}packages/astro/src/react/controlComponents.tsx (3)
52-60: UnusedPropsWithChildrenimportAfter switching to
React.PropsWithChildren, thePropsWithChildrenimport at line 3 is now unused. Remove it to keep the file lint-clean.
88-109:Protectdestructuring refactor is fine, but duplicate vars could be inlined
const unauthorized = …,const authorized = children;are immediately returned; consider inlining to reduce indirection, but this is purely stylistic.
145-146: Use OXLint directive instead of ESLint if ESLint is being phased outThe comment still disables
react-hooks/exhaustive-depsfor ESLint. If OXLint replaces ESLint, convert or remove this directive to avoid orphaned comments.packages/shared/scripts/postinstall.mjs (2)
50-54: Silent catch may mask corruption of telemetry configSilently ignoring
JSON.parseerrors means a corruptedconfig.jsonis perpetually discarded and rewritten, destroying any existing (possibly user-edited) settings.try { config = JSON.parse(await fs.readFile(configFile, 'utf8')); -} catch { - // File can't be read and parsed, continue +} catch { + // File can't be read or parsed → keep defaults but surface a debug log + if (process.env.DEBUG?.includes('clerk-postinstall')) { + console.warn('[clerk] telemetry config unreadable – resetting to defaults'); + } }This keeps the install noise at zero yet allows opt-in diagnostics when needed.
70-74: Same concern for the outertry…catchA suppressed error here hides unexpected I/O issues (e.g. permission problems).
At minimum, consider logging behind aDEBUGguard as above.package.json (1)
25-25: Script name diverges from package-level conventionEvery workspace package exposes
lint:oxlint, but the root exposes plainoxlint.
For symmetry (and muscle-memory), add an alias:"lint:publint": "FORCE_COLOR=1 turbo lint:publint", + "lint:oxlint": "pnpm oxlint", "nuke": "node ./scripts/nuke.mjs", "oxlint": "oxlint --quiet",.oxlintrc.json (7)
1-6: Consider extending shared or ESLint configs.Instead of enumerating all plugins and disabling the entire
correctnesscategory, you could leverage anextendsfield to reuse baseline rules from a shared or ESLint config and then override only the exceptions.
105-133: Revisit TypeScript rule exceptions.Disabling
@typescript-eslint/no-explicit-anyand turning off"no-throw-literal"may mask unsafe patterns. Consider re-enabling them with fine-grained allowances or documenting why they’re off.
241-261: Consolidate TypeScript overrides.Many rules are toggled off for TS files. Consider grouping these toggles under a shared base config or merging with the main overrides to reduce duplication.
263-277: Review JS override for unused vars.You’ve redefined
no-unused-varsfor JS files. If this matches the root config, you can inherit it instead of duplicating.
278-283: Simplify ES2018 environment override.If all TS targets run on ES2018, merge this env block with the earlier TS override to cut down on fragmentation.
308-313: Confirm exception for upgrade package.Disabling
react/no-unescaped-entitiesinpackages/upgradelooks like a quick workaround. Consider adding a TODO or filing an issue if this isn’t a permanent exception.
314-381: Abstract integration test globals.Listing 70+ globals inline bloats the config. Consider moving them to a shared
globals.jsonor leveraging a preset environment to simplify maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
.oxlintrc.json(1 hunks)package.json(2 hunks)packages/agent-toolkit/package.json(1 hunks)packages/astro/package.json(1 hunks)packages/astro/src/astro-components/control/ProtectCSR.astro(1 hunks)packages/astro/src/react/controlComponents.tsx(4 hunks)packages/backend/package.json(1 hunks)packages/backend/src/runtime/node/crypto.js(1 hunks)packages/chrome-extension/package.json(1 hunks)packages/clerk-js/headless/index.js(1 hunks)packages/clerk-js/no-rhc/index.js(1 hunks)packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts(3 hunks)packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts(1 hunks)packages/clerk-js/src/core/resources/Organization.ts(6 hunks)packages/clerk-js/src/core/resources/OrganizationMembership.ts(1 hunks)packages/clerk-js/src/core/resources/OrganizationSuggestion.ts(1 hunks)packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts(1 hunks)packages/clerk-js/src/utils/url.ts(2 hunks)packages/clerk-js/svgTransform.js(1 hunks)packages/dev-cli/package.json(1 hunks)packages/elements/package.json(1 hunks)packages/expo-passkeys/package.json(1 hunks)packages/expo/package.json(1 hunks)packages/express/package.json(1 hunks)packages/fastify/package.json(1 hunks)packages/localizations/package.json(1 hunks)packages/nextjs/package.json(1 hunks)packages/nuxt/package.json(1 hunks)packages/react-router/package.json(1 hunks)packages/react/package.json(1 hunks)packages/remix/package.json(1 hunks)packages/shared/package.json(1 hunks)packages/shared/scripts/postinstall.mjs(1 hunks)packages/tanstack-react-start/package.json(1 hunks)packages/testing/package.json(1 hunks)packages/themes/package.json(1 hunks)packages/types/package.json(1 hunks)packages/upgrade/package.json(1 hunks)packages/vue/package.json(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/clerk-js/src/core/resources/OrganizationSuggestion.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
OrganizationSuggestionJSON(464-471)
packages/clerk-js/src/core/resources/OrganizationMembership.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
OrganizationMembershipJSON(387-398)
packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
UserOrganizationInvitationJSON(483-493)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (2)
CommerceSubscriptionJSON(684-698)CommerceStatementJSON(655-662)
packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
CommercePaymentSourceJSON(636-646)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (44)
packages/clerk-js/headless/index.js (1)
1-2: Confirm Oxlint disable directive effectiveness
Ensure OXLint recognizes this// oxlint-disable-next-line @typescript-eslint/no-require-importsdirective and properly suppresses the rule on the following require statement.packages/clerk-js/svgTransform.js (1)
2-2: Approve OXLint suppression for require imports
The inline// oxlint-disable-line @typescript-eslint/no-require-importscorrectly suppresses the rule on therequire('path')line.packages/clerk-js/no-rhc/index.js (1)
1-2: Confirm Oxlint disable directive effectiveness
Ensure OXLint recognizes this// oxlint-disable-next-line @typescript-eslint/no-require-importsdirective and properly suppresses the rule on the subsequentrequirestatement.packages/clerk-js/src/core/resources/OrganizationMembership.ts (1)
42-44: Verify rule ID for Oxlint directive
The inline// oxlint-disable-line no-unsafe-optional-chainingomits the@typescript-eslintprefix. Confirm whether Oxlint’s rule identifier isno-unsafe-optional-chainingor should be@typescript-eslint/no-unsafe-optional-chainingto ensure the suppression takes effect.packages/astro/src/react/controlComponents.tsx (1)
62-70: Return type update looks goodChanging the return type to
React.ReactNodeand dropping the fragment wrapper is correct and simplifies the component. Nice cleanup.packages/nextjs/package.json (1)
69-70: Script added correctly
lint:oxlintscript is wired properly and scoped tosrc. No issues spotted.packages/shared/package.json (1)
137-138: Consistent workspace script – good
lint:oxlintmirrors the pattern used elsewhere, keeping tooling uniform across packages.packages/tanstack-react-start/package.json (1)
64-64: Introducelint:oxlintscript
Added thelint:oxlintcommand to run Oxlint quietly on thesrcdirectory, in line with the PR’s goal to integrate Oxlint alongside ESLint for faster lint feedback.packages/agent-toolkit/package.json (1)
44-44: Add Oxlint linting step
The newlint:oxlintscript runs Oxlint in quiet mode againstsrc, standardizing the lint workflow across packages as described in the PR summary.packages/backend/package.json (1)
98-98: Enable Oxlint for the backend package
Introduced thelint:oxlintscript to run Oxlint quietly onsrc, aligning with the monorepo-wide Oxlint rollout.packages/themes/package.json (1)
38-39: Add and formatlint:oxlintentry
Inserted thelint:oxlintscript (quiet mode onsrc) and added the necessary comma on the preceding line, ensuring consistency with other packages’ lint configurations.packages/astro/package.json (1)
85-85: Configure Oxlint for Astro integration
Addedlint:oxlintto target bothsrcandenv.d.ts, matching the PR’s aim to include Oxlint across all packages. Verify this matches the root.oxlintrc.jsonsettings.packages/types/package.json (1)
39-39: Addlint:oxlintscript for OXLint
The newlint:oxlintscript matches the pattern used across other packages and correctly targets thesrcdirectory with--quiet.packages/elements/package.json (1)
68-68: Addlint:oxlintscript for OXLint
This mirrors the addition in other packages and ensures OXLint runs quietly againstsrc. Looks consistent and correct.packages/remix/package.json (1)
72-72: Addlint:oxlintscript for OXLint
The new script is consistent with the monorepo’s OXLint adoption and will run quietly on thesrcfolder.packages/expo/package.json (1)
75-75: Addlint:oxlintscript for OXLint
This addition aligns with the other package scripts and correctly invokes OXLint in quiet mode onsrc.packages/nuxt/package.json (1)
64-64: Addlint:oxlintscript for OXLint
The script follows the established pattern for OXLint in the monorepo, targetingsrcwith--quiet.packages/clerk-js/package.json (1)
49-49: Addlint:oxlintscript
The newlint:oxlintcommand aligns with the monorepo’s linting strategy and correctly targets thesrcdirectory.packages/chrome-extension/package.json (1)
43-43: Addlint:oxlintscript
Introducinglint:oxlinthere mirrors other packages and ensures consistent lint checks on thesrcfolder.packages/expo-passkeys/package.json (1)
29-29: Addlint:oxlintscript
This addition standardizes Oxlint usage across packages and correctly points at the source directory.packages/fastify/package.json (1)
60-60: Addlint:oxlintscript
Adding Oxlint here aligns with the updated Turbo pipeline and ensures fast feedback for Fastify sources.packages/dev-cli/package.json (1)
23-23: Addlint:oxlintscript
The new script complements the existing ESLint step and brings Oxlint checks into the CLI package.packages/react/package.json (1)
73-73: Addition of OXLint lint script
The newlint:oxlintentry aligns with existing lint scripts and ensures OXLint runs againstsrc. Confirm that this directory exists and that your root.oxlintrc.jsoncovers this package.packages/express/package.json (1)
62-62: Addlint:oxlintfor Express package
Thelint:oxlintscript is correctly introduced alongside other lint targets. Verify thatsrcis present and.oxlintrc.jsonrules apply here.packages/vue/package.json (1)
53-53: Introduce OXLint script to Vue package
Looks good and consistent with other packages. Ensure thesrcfolder exists and is picked up by OXLint per your root config.packages/react-router/package.json (1)
77-77: OXLint integration for React Router
The newlint:oxlintscript matches the repo convention. Double-check that.oxlintrc.jsonrules cover this package’ssrcdirectory.packages/testing/package.json (1)
73-73: Enable OXLint for testing utilities
Addinglint:oxlinthere maintains consistency. Confirmsrcexists underpackages/testingand the root config applies.packages/localizations/package.json (1)
103-106:lint:oxlintscript added – looks correctThe command restricts the run to
srcand uses--quiet, mirroring the pattern adopted in other packages. ✅packages/upgrade/package.json (1)
22-24: Consistentlint:oxlintscriptSame convention as the rest of the monorepo; nothing further.
package.json (1)
124-124: Pinning with^risks auto-upgrading a tool that can fail CIGiven OXLint is new and the monorepo is large, consider a fixed version (
"1.0.0"without the caret) until the team is confident with its release cadence.turbo.json (1)
104-124: New top-levellinttask overlaps with existing rootlintscript
package.json#scripts.lintalready points tonode ./scripts/lint.mjs.
That script may callturbointernally, but the identical task name insideturbo.jsoncan cause confusion when reading the config or invokingturbo run lint.Double-check that:
scripts/lint.mjsstill behaves as intended (does it now recurse into the new task and double-run?).- CI pipelines and docs aren’t relying on
turbo run lintmeaning “ESLint only”.If aliasing is intentional, add a comment here or in the script for clarity.
.oxlintrc.json (13)
7-11: Verify environment settings.Confirm that enabling
"builtin","es2021", and"commonjs"covers all your target runtimes; otherwise you may miss globals (e.g., browser or Node 18+ features).
12-40: Review ignore patterns for edge cases.The
"!.*.js"negation and various globs might unintentionally include hidden files or skip generated artifacts. Please test that build outputs and source files are correctly included/excluded as intended.
41-104: Audit core rule overrides.Strict JS rules are good, but you’ve set
"no-unexpected-multiline"to"off". Ensure this exception is intentional, as it can allow confusing control flows.
134-138: Approve import plugin rules.Import checks (
namespace,default,no-named-as-default,no-duplicates) are well configured to catch common module issues.
139-171: Validate accessibility rule coverage.Your JSX A11y rules are comprehensive, but warnings like
html-has-langandno-autofocuscan be tightened or relaxed per your design system. Confirm these align with your accessibility policy.
172-196: Confirm React rule choices.Disabling
react/react-in-jsx-scoperequires React 17+. Ensure all packages use a compatible React version.
197-239: Approve stylistic and hook rules.Curly/style rules, import sorting, and React hooks settings are sensibly set to
offorwarnper your team conventions.
284-290: Check non-null assertion rule overlap.You enforce
@typescript-eslint/no-non-null-assertionhere, but also haveno-unsafe-optional-chainingglobally. Verify there’s no conflict or redundant coverage.
291-296: Approve Jest environment override.Scoping the Jest env to test file patterns is correct.
297-306: Verify restricted imports in expo-passkeys.Blocking
node:*imports makes sense—double-check if any other core modules should be prohibited in that package.
382-416: Validate JSDoc plugin setup.Your JSDoc rules for
shared/srcare thorough. Make sure they align with your actual documentation standards and that the team is prepared to maintain these additional checks.
417-423: Approve YAML overrides.Disabling whitespace and unused-vars checks for YAML is appropriate.
424-429: Confirm compiled code exceptions.Excluding unused-expression checks for the compiled
path-to-regexpfile is fine; ensure future regenerations don’t introduce new rule violations outside this scope.
| const { data: suggestions, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against undefined responses instead of silencing the lint rule
res?.response can legally be undefined if _fetch returns undefined (e.g. network failure).
Destructuring from an undefined value will throw at runtime, so disabling no-unsafe-optional-chaining hides a potential crash.
-const { data: suggestions, total_count } =
- res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining
+if (!res?.response) {
+ throw new Error('OrganizationSuggestion.retrieve: empty response');
+}
+const { data: suggestions, total_count } =
+ res.response as ClerkPaginatedResponse<OrganizationSuggestionJSON>;Alternatively, tighten the _fetch return type so callers no longer need ?. or double as casts.
🏁 Script executed:
#!/bin/bash
set -e
echo "Searching for _fetch occurrences in core resources:"
rg "_fetch" -n packages/clerk-js/src/core/resources/
echo ""
echo "Showing OrganizationSuggestion.ts around the fetch call:"
rg -n -C5 "res?.response" -n packages/clerk-js/src/core/resources/OrganizationSuggestion.tsLength of output: 9301
🏁 Script executed:
#!/bin/bash
set -e
echo "Displaying lines 1-80 of OrganizationSuggestion.ts for context:"
sed -n '1,80p' packages/clerk-js/src/core/resources/OrganizationSuggestion.tsLength of output: 2453
🏁 Script executed:
#!/bin/bash
set -e
echo "Locating _fetch signature in Base.ts:"
rg "static async _fetch" -n packages/clerk-js/src/core/resources/Base.ts
echo ""
echo "Displaying BaseResource._fetch implementation (lines 1–200):"
sed -n '1,200p' packages/clerk-js/src/core/resources/Base.tsLength of output: 6892
Guard against undefined res.response before destructuring
Destructuring from an undefined response will crash at runtime if _fetch returns null (e.g. offline/browser network failure). Replace the unsafe optional chaining and cast with an explicit null check:
packages/clerk-js/src/core/resources/OrganizationSuggestion.ts – in retrieve:
-return await BaseResource._fetch({
- path: '/me/organization_suggestions',
- method: 'GET',
- search: convertPageToOffsetSearchParams(params),
-}).then(res => {
- const { data: suggestions, total_count } =
- res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining
-
- return {
- total_count,
- data: suggestions.map(suggestion => new OrganizationSuggestion(suggestion)),
- };
-});
+return await BaseResource._fetch({
+ path: '/me/organization_suggestions',
+ method: 'GET',
+ search: convertPageToOffsetSearchParams(params),
+}).then(res => {
+ if (!res || !res.response) {
+ throw new Error('OrganizationSuggestion.retrieve: empty or null response');
+ }
+ const { data: suggestions, total_count } =
+ res.response as ClerkPaginatedResponse<OrganizationSuggestionJSON>;
+
+ return {
+ total_count,
+ data: suggestions.map(suggestion => new OrganizationSuggestion(suggestion)),
+ };
+});This change throws a clear error on missing responses instead of risking an uncaught TypeError.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: suggestions, total_count } = | |
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; | |
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | |
| return await BaseResource._fetch({ | |
| path: '/me/organization_suggestions', | |
| method: 'GET', | |
| search: convertPageToOffsetSearchParams(params), | |
| }).then(res => { | |
| if (!res || !res.response) { | |
| throw new Error('OrganizationSuggestion.retrieve: empty or null response'); | |
| } | |
| const { data: suggestions, total_count } = | |
| res.response as ClerkPaginatedResponse<OrganizationSuggestionJSON>; | |
| return { | |
| total_count, | |
| data: suggestions.map(suggestion => new OrganizationSuggestion(suggestion)), | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/OrganizationSuggestion.ts around lines
34 to 36, the code destructures properties from res?.response using unsafe
optional chaining and double casting, which can cause a runtime crash if
res.response is undefined due to network failure. To fix this, add an explicit
null or undefined check for res.response before destructuring, and throw a clear
error if it is missing, removing the unsafe optional chaining and double casting
to ensure safer and more predictable behavior.
| ).then(res => { | ||
| // oxlint-disable-next-line no-unsafe-optional-chaining | ||
| const { data: roles, total_count } = res?.response as unknown as ClerkPaginatedResponse<RoleJSON>; | ||
|
|
||
| return { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer runtime guard or stronger typings over oxlint-disable-next-line
If _fetch guarantees a response, drop ? and the disable comment; if not, guard first.
Repeated across this file — consider a helper to unpack paginated responses to avoid boilerplate & scattered ignores.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/Organization.ts around lines 100 to 104,
the code uses optional chaining with a disable comment to bypass linting on
unsafe access to res.response. To fix this, first confirm if _fetch always
returns a response; if so, remove the optional chaining and the disable comment.
If not guaranteed, add a runtime check to safely access res.response before
destructuring. Additionally, refactor by creating a helper function to extract
and validate paginated responses to reduce repeated boilerplate and avoid
scattered lint disables throughout the file.
| }).then(res => { | ||
| const { data: subscriptions, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
res?.response can still blow up at runtime – better to assert non-null than silence OXLint
You’re suppressing no-unsafe-optional-chaining, but if _fetch ever returns undefined the destructuring on the next line (const { data, total_count } = …) will throw anyway.
Either assert non-null (e.g. invariant(res)) or propagate an error before destructuring instead of keeping the optional chain and disabling the rule.
-const { data: subscriptions, total_count } =
- res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; // oxlint-disable-line no-unsafe-optional-chaining
+if (!res) {
+ throw new Error('getSubscriptions: unexpected empty response');
+}
+const { data: subscriptions, total_count } =
+ (res.response as ClerkPaginatedResponse<CommerceSubscriptionJSON>);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }).then(res => { | |
| const { data: subscriptions, total_count } = | |
| res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; | |
| res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | |
| }).then(res => { | |
| if (!res) { | |
| throw new Error('getSubscriptions: unexpected empty response'); | |
| } | |
| const { data: subscriptions, total_count } = | |
| (res.response as ClerkPaginatedResponse<CommerceSubscriptionJSON>); |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts around lines
51 to 54, the code uses optional chaining on res.response but disables the
no-unsafe-optional-chaining lint rule, which can still cause runtime errors if
res is undefined. To fix this, add a non-null assertion or explicitly check that
res is not null or undefined before destructuring its response property, and
remove the lint disable comment. This ensures safe access and prevents runtime
crashes.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
packages/expo-passkeys/package.json (1)
29-29: Lint script addition aligns with other packages
The newlint:oxlintscript matches the pattern used across the repo. Ensure your CI and Turbo workflows pick up this command and that the root.oxlintrc.jsonapplies correctly in this package.packages/themes/package.json (1)
38-39: Lint script addition aligns with other packages
Thelint:oxlintentry follows the established convention. Verify that the root-level OXLint config and any package-specific overrides behave as expected here.packages/nuxt/package.json (1)
64-64: Lint script addition aligns with other packages
This matches thelint:oxlintconvention introduced elsewhere. Confirm that the CI/Turbo pipeline invokes it correctly for the Nuxt package.packages/testing/package.json (1)
73-73: Lint script addition aligns with other packages
The newlint:oxlintentry is consistent with the monorepo’s linting approach. Ensure test packages also respect the shared.oxlintrc.jsonrules.
🧹 Nitpick comments (10)
packages/shared/scripts/postinstall.mjs (1)
50-54: Silent catch blocks hinder troubleshootingBoth
catch {}blocks swallow all errors without even a debug-level log.
Silent failures make telemetry-notice issues extremely hard to diagnose.Consider at least logging behind an env flag:
-} catch { - // File can't be read and parsed, continue +} catch (err) { + if (process.env.CLERK_POSTINSTALL_DEBUG === '1') { + console.warn('[clerk/postinstall] telemetry config read failed:', err); + } }This keeps install output clean while giving maintainers a way to surface issues on demand.
packages/clerk-js/src/utils/url.ts (2)
247-252: Null-byte check duplicated & extra pragma noise
/[\x00-\x1F]/already matches\0, so the separate/\0/entry (and its extrano-control-regexdisable) is redundant. Consolidating reduces rule-disable clutter.-// oxlint-disable-next-line no-control-regex -/\0/, // Null bytes -... -// eslint-disable-next-line no-control-regex -/[\x00-\x1F]/, // Control characters +// eslint-disable-next-line no-control-regex +/[\x00-\x1F]/, // Control + null bytes🧰 Tools
🪛 Biome (1.9.4)
[error] 251-251: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 251-251: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
263-266: Redundant explicit pathname'\0'checkThe regex (after consolidation) already catches null bytes in the pathname; this extra
includes('\0')branch can be removed for brevity unless you have a benchmark demonstrating a measurable gain.packages/astro/src/astro-components/control/ProtectCSR.astro (1)
47-53: Avoid side-effect ternaries and drop the lint suppressionThe ternary is used solely for its side effect, which is precisely what
no-unused-expressionstries to prevent.
Keeping the suppression leaves hard-to-scan logic and increases reviewer cognitive load.-// oxlint-disable-next-line no-unused-expressions -isUnauthorized ? this.defaultSlot.setAttribute('hidden', '') : this.defaultSlot.removeAttribute('hidden'); +if (isUnauthorized) { + this.defaultSlot.setAttribute('hidden', ''); +} else { + this.defaultSlot.removeAttribute('hidden'); +}Apply the same pattern to
fallbackSlotbelow and drop bothoxlint-disablecomments.package.json (1)
25-25: Script name inconsistencyMost packages expose
lint:oxlint, but the root adds plainoxlint.
For discoverability and parity with sub-packages, consider renaming:-"oxlint": "oxlint --quiet", +"lint:oxlint": "oxlint --quiet",Update CI or Turbo tasks if they call the old name.
turbo.json (1)
104-124: Circular-ish dependency risk:lint→lint:oxlint(which waits for^build)
lintnow waits forlint:oxlint;lint:oxlintwaits for all upstreambuildtasks.
If any package’sbuilddepends onlint, you’ll create a cycle.
Double-check the task graph; if safe today, document the constraint to avoid future cycles..oxlintrc.json (4)
3-3: Consider adding the JSDoc plugin globally.You currently only load
"jsdoc"under a specific override. If your codebase relies on JSDoc annotations broadly, you may want to include"jsdoc"in the top‐levelpluginsarray to catch issues outsidepackages/shared.
7-11: Add a browser/global environment flag.Root
envincludesbuiltin,es2021, andcommonjsbut omitsbrowser. Linting React and web packages without a browser env may lead to false positives for globals likewindowordocument. Consider either adding"browser": truehere or in frontend-specific overrides.
12-40: Expand and validate your ignore patterns.
- Missing common lockfiles like
yarn.lockorpackage-lock.json.- The negation pattern
"!.*.js"can be confusing; verify it has the intended effect.- You may also want to ignore
.husky/or other tooling directories if present.
141-180: Align JSX-A11y severity levels.Some accessibility rules (e.g.,
jsx-a11y/html-has-lang) are warnings while others are errors. Consider standardizing severity (all as errors or warnings) based on your accessibility compliance goals.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
.oxlintrc.json(1 hunks)package.json(2 hunks)packages/agent-toolkit/package.json(1 hunks)packages/astro/package.json(1 hunks)packages/astro/src/astro-components/control/ProtectCSR.astro(1 hunks)packages/astro/src/react/controlComponents.tsx(4 hunks)packages/backend/package.json(1 hunks)packages/backend/src/runtime/node/crypto.js(1 hunks)packages/chrome-extension/package.json(1 hunks)packages/clerk-js/headless/index.js(1 hunks)packages/clerk-js/no-rhc/index.js(1 hunks)packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts(3 hunks)packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts(1 hunks)packages/clerk-js/src/core/resources/Organization.ts(6 hunks)packages/clerk-js/src/core/resources/OrganizationMembership.ts(1 hunks)packages/clerk-js/src/core/resources/OrganizationSuggestion.ts(1 hunks)packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts(1 hunks)packages/clerk-js/src/utils/url.ts(2 hunks)packages/clerk-js/svgTransform.js(1 hunks)packages/dev-cli/package.json(1 hunks)packages/elements/package.json(1 hunks)packages/expo-passkeys/package.json(1 hunks)packages/expo/package.json(1 hunks)packages/express/package.json(1 hunks)packages/fastify/package.json(1 hunks)packages/localizations/package.json(1 hunks)packages/nextjs/package.json(1 hunks)packages/nuxt/package.json(1 hunks)packages/react-router/package.json(1 hunks)packages/react/package.json(1 hunks)packages/remix/package.json(1 hunks)packages/shared/package.json(1 hunks)packages/shared/scripts/postinstall.mjs(1 hunks)packages/tanstack-react-start/package.json(1 hunks)packages/testing/package.json(1 hunks)packages/themes/package.json(1 hunks)packages/types/package.json(1 hunks)packages/upgrade/package.json(1 hunks)packages/vue/package.json(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
UserOrganizationInvitationJSON(483-493)
packages/clerk-js/src/core/resources/OrganizationMembership.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
OrganizationMembershipJSON(387-398)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (2)
CommerceSubscriptionJSON(684-698)CommerceStatementJSON(655-662)
packages/clerk-js/src/core/resources/Organization.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (4)
OrganizationMembershipRequestJSON(473-481)OrganizationMembershipJSON(387-398)OrganizationInvitationJSON(400-411)CommerceSubscriptionJSON(684-698)
packages/clerk-js/src/core/resources/OrganizationSuggestion.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
OrganizationSuggestionJSON(464-471)
packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
CommercePaymentSourceJSON(636-646)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (47)
packages/clerk-js/headless/index.js (1)
1-1: Approve lint suppression
The added// oxlint-disable-next-line @typescript-eslint/no-require-importsdirective is correct for this CommonJS module export, matching the new OXLint rules and existing annotations across the codebase.packages/clerk-js/no-rhc/index.js (1)
1-1: Approve lint suppression
This// oxlint-disable-next-line @typescript-eslint/no-require-importscomment properly disables the require-imports rule for the subsequent CommonJS export, keeping consistency with other packages.packages/clerk-js/svgTransform.js (1)
2-2: Approve lint disable for require-imports
The inline// oxlint-disable-line @typescript-eslint/no-require-importson therequire('path')import is appropriate and aligns with project-wide lint exemptions for legacy CommonJS patterns.packages/backend/src/runtime/node/crypto.js (1)
1-1: Approve lint suppression
Disabling@typescript-eslint/no-require-importshere accurately accommodates the CommonJSrequireusage under the new OXLint configuration.packages/clerk-js/src/core/resources/OrganizationMembership.ts (1)
43-43: Approve unsafe-optional-chaining suppression
The// oxlint-disable-line no-unsafe-optional-chainingdirective is justified for the cast onres?.responseand maintains parity with similar suppressions in other resource files.packages/astro/src/react/controlComponents.tsx (4)
52-60: Type widening toReact.ReactNodeis sensibleReturning
childrendirectly (without fragment wrapping) and widening the return type improves flexibility and removes unnecessary JSX. Looks good.
62-70: Symmetric change inClerkLoadingalso LGTMConsistent with the above update; no issues spotted.
88-109: Simplified authorized / unauthorized branches read clearerRemoving unnecessary fragments keeps the JSX minimal.
145-146: Empty dependency array suppression: verify intentSuppressing
react-hooks/exhaustive-depsis fine ifhandleRedirectCallbackmust run exactly once, but ensure that props (clerk,handleRedirectCallbackParams) are immutable for the component’s lifetime; otherwise the effect may miss updates.packages/shared/package.json (1)
137-137: Added lint:oxlint script for OXLint integration
This aligns with the monorepo’s new linting workflow and mirrors other packages.packages/react-router/package.json (1)
77-77: Introduce lint:oxlint script
Matches the pattern used across other packages to run OXLint in quiet mode onsrc.packages/backend/package.json (1)
98-98: Add lint:oxlint to scripts
Consistent with the new OXLint-based linting setup; ready to catch issues faster.packages/nextjs/package.json (1)
69-69: Add lint:oxlint script
Ensures NextJS package participates in the unified OXLint workflow.packages/agent-toolkit/package.json (1)
44-44: Include lint:oxlint command
Aligns agent-toolkit with the repo-wide OXLint configuration.packages/remix/package.json (1)
72-72: Approve: Addlint:oxlintscript.
This aligns with the repo-wide OXLint integration and matches other packages.packages/types/package.json (1)
39-39: Approve: Addlint:oxlintscript.
Consistent with the new OXLint setup across workspace packages.packages/expo/package.json (1)
75-75: Approve: Addlint:oxlintscript.
Follows the monorepo convention for OXLint and keeps parity with ESLint scripts.packages/upgrade/package.json (1)
23-23: Approve: Addlint:oxlintscript.
Integrates OXLint into the upgrade package as intended.packages/react/package.json (1)
73-73: Approve: Addlint:oxlintscript.
Matches the new linting workflow and other package.json scripts.packages/tanstack-react-start/package.json (1)
64-64: Addlint:oxlintscript
Consistently adds OXLint linting for thesrcdirectory, aligning with the new linting workflow.packages/clerk-js/package.json (1)
49-49: Addlint:oxlintscript
Introduces OXLint linting for thesrcdirectory, matching the monorepo’s standardized lint setup.packages/astro/package.json (1)
85-85: Addlint:oxlintscript
Runs OXLint quietly on bothsrcandenv.d.ts, consistent with other packages’ lint configurations.packages/dev-cli/package.json (1)
23-23: Addlint:oxlintscript
Adds OXLint linting for thesrcdirectory, aligning this CLI tool with the repo’s new lint pipeline.packages/fastify/package.json (1)
60-60: Addlint:oxlintscript
Introduces OXLint linting for thesrcdirectory, ensuring consistent lint coverage across packages.packages/chrome-extension/package.json (1)
43-43: Introducelint:oxlintscript for OXLint integration
This addition aligns the Chrome extension package with the new monorepo-wide OXLint setup. Ensure thatoxlintis installed and resolvable in this workspace (hoisted devDependency or local install).packages/localizations/package.json (1)
104-105: Addlint:oxlintalongside existing scripts
Consistent with other packages, this enables OXLint in quiet mode on thesrcdirectory. The trailing comma on thelint:attwentry is also correct.packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts (1)
49-49: Suppress OXLint’s no-unsafe-optional-chaining rule locally
The inline// oxlint-disable-line no-unsafe-optional-chainingis necessary to avoid false positives on this known-safe access.packages/vue/package.json (1)
53-53: Introducelint:oxlintscript to Vue package
This mirrors the monorepo’s new OXLint configuration and ensures the Vue package is linted by Oxlint as well.packages/express/package.json (1)
62-62: Addlint:oxlintscript for Express SDK
Aligns the Express package with the project’s OXLint integration, enabling faster lint failures in CI.packages/elements/package.json (1)
68-68:❓ Verification inconclusive
Ensure OXLint CLI availability
Confirm that theoxlintbinary is resolvable within this package’s context (via workspace hoisting). If you rely on root-level devDependencies, add a verification step or document the requirement.Run:
to verify the CLI is available and returns the expected version.
🏁 Script executed:
pnpm -C packages/elements oxlint --versionLength of output: 129
🏁 Script executed:
#!/bin/bash # Search for OXLint in all package.json files to verify where it’s declared rg -g 'package.json' -n '"oxlint"'Length of output: 123
Ensure OXLint CLI availability in packages/elements
File: packages/elements/package.json
Lines: 68"lint:oxlint": "oxlint --quiet src",The
lint:oxlintscript assumes theoxlintbinary is available in this package. Although it’s declared in the rootpackage.json, PNPM workspaces may hoist it—but you need to confirm.
- Run:
to verify it prints the expected version (e.g.pnpm -C packages/elements exec -- oxlint --version1.0.0).- If that fails, either add
to"devDependencies": { "oxlint": "^1.0.0" }packages/elements/package.json, or document that this script requires the root’s devDependencies.package.json (1)
124-124: Pin OXLint to an explicit minor/patch to avoid unexpected breakage
"^1.0.0"will pull in future breaking minors if the semver discipline slips.
Given OXLint is new, pin to1.0.xor exact1.0.0until confidence grows..oxlintrc.json (16)
2-2: Verify the$schemapath is correct.Ensure that
./node_modules/oxlint/configuration_schema.jsonmatches the installed OXLint package layout and version. An incorrect schema path could break IDE integrations or CLI validation.
4-6: Double‐check disabling the entire “correctness” category.Turning off the
"correctness"category may inadvertently suppress critical runtime checks. Confirm that you’re re-enabling all needed correctness rules individually, and that none are unintentionally disabled by the category override.
42-104: Review core rule disables and warnings.You’ve turned off
no-unexpected-multiline(line 82) and set some rules to"warn". Confirm these exceptions align with your team’s quality targets. If these are temporary, consider adding TODOs for follow-up.
105-140: Audit TypeScript and import rules.
@typescript-eslint/no-explicit-anyis globally"off"; ensure this aligns with your strategy for type safety.- Ensure import rules (
import/namespace,import/default, etc.) cover your existing code patterns without causing false positives.
181-240: Verify React and Unicorn rule choices.
react/react-in-jsx-scopeis correctly off for modern React.- A few
unicornrules (e.g.,no-nested-ternary) are disabled—confirm these are deliberate trade-offs.
241-262: Approve TypeScript override adjustments.Disabling certain base rules in TS files while enforcing
no-varandprefer-spreadis a solid approach to avoid false positives.
263-277: Approve JS override for unused vars.Customizing
no-unused-varsfor JS/JSX to match the TS pattern maintains consistency across file types.
278-283: Reevaluate the ES2018 override.Root env is already
es2021; you may not need an extra ES2018 override unless targeting specific build outputs. Confirm its necessity.
284-290: Approve TS non-null assertion rule.Turning off base
no-unused-varsand enforcing@typescript-eslint/no-non-null-assertionat error level is consistent.
291-296: Approve Jest environment for tests.Enabling
jestglobals in test files is correct.
297-307: Approve node: import restriction.*Disallowing
node:*imports inexpo-passkeysaligns with platform constraints.
308-313: Approve React unescaped entities override.Disabling
react/no-unescaped-entitiesforpackages/upgradeis acceptable if this package relies on literal HTML entities.
314-381: Confirm integration test globals are complete.You’ve declared an extensive list of globals—verify that no important global (e.g.,
process,__dirname, orBuffer) is missing and consider adding a Node env override if required.
382-416: Approve JSDoc rules for shared sources.Scoped JSDoc linting in
packages/sharedhelps enforce documentation standards without impacting the rest of the repo.
417-423: Approve YAML file overrides.Disabling whitespace and unused-vars rules for YAML is standard to prevent false positives.
424-429: Approve compiled code override.Exempting
no-unused-expressionsin compiled sources is appropriate to avoid linting generated artifacts.
| const { data: suggestions, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
Unsafe optional-chaining still leads to a potential runtime crash
res?.response may be undefined, yet the next line immediately destructures data & total_count.
If res or res.response is missing the code will throw “Cannot destructure property 'data' of 'undefined'” at runtime.
Disabling the lint rule masks a real bug rather than a false-positive.
-const { data: suggestions, total_count } =
- res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining
+const response = res?.response as ClerkPaginatedResponse<OrganizationSuggestionJSON> | undefined;
+if (!response) {
+ throw new Error('Unexpected empty response from /me/organization_suggestions');
+}
+const { data: suggestions, total_count } = response;Remove the oxlint-disable-line once the code is actually safe.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: suggestions, total_count } = | |
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; | |
| res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | |
| const response = res?.response as ClerkPaginatedResponse<OrganizationSuggestionJSON> | undefined; | |
| if (!response) { | |
| throw new Error('Unexpected empty response from /me/organization_suggestions'); | |
| } | |
| const { data: suggestions, total_count } = response; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/OrganizationSuggestion.ts around lines
34 to 36, the code uses optional chaining on res?.response but immediately
destructures data and total_count without ensuring res.response is defined,
risking a runtime crash. To fix this, add a check to confirm res and
res.response are defined before destructuring, or provide a safe fallback value
to avoid destructuring undefined. Remove the oxlint-disable-line comment once
this safety is ensured.
| const { data: invites, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<UserOrganizationInvitationJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<UserOrganizationInvitationJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
Same unsafe pattern – destructuring on possibly undefined value
Identical to the issue in OrganizationSuggestion.ts: res?.response can be undefined, causing a crash during destructuring. Please add an explicit guard instead of suppressing the rule.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts around
lines 32 to 34, the code destructures properties from res?.response without
checking if it is defined, which can cause a runtime error. To fix this, add an
explicit guard to verify that res?.response is not undefined before
destructuring. If it is undefined, handle that case appropriately to avoid
crashes instead of suppressing the lint rule.
| const { data: statements, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<CommerceStatementJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<CommerceStatementJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same safety fix to getStatements and getPaymentAttempts
Both blocks repeat the unsafe pattern. Refactor them similarly or extract a helper (parsePaginated(res)) to DRY it out.
Also applies to: 90-92
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts around lines
70-72 and 90-92, the code uses unsafe optional chaining and type assertions to
extract paginated response data from res.response. Refactor both getStatements
and getPaymentAttempts to safely parse the paginated response by either applying
the same safety checks or extracting a helper function like parsePaginated(res)
that handles the type assertion and optional chaining safely, then use this
helper in both methods to avoid code duplication and improve safety.
| }).then(res => { | ||
| const { data: subscriptions, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
res?.response will still crash on undefined – the lint rule was right
const { data, total_count } = res?.response … will throw a TypeError when res is undefined because destructuring happens after the optional-chaining. Silencing no-unsafe-optional-chaining hides a real runtime risk.
-return await BaseResource._fetch({ ... }).then(res => {
- const { data: subscriptions, total_count } =
- res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>;
+return await BaseResource._fetch({ ... }).then(res => {
+ if (!res || !res.response) {
+ return { total_count: 0, data: [] };
+ }
+ const { data: subscriptions, total_count } =
+ res.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>;Either assert _fetch always resolves to a response object or guard as above, then remove the lint-disable.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts around lines
51 to 54, the destructuring of res?.response can cause a TypeError if res is
undefined because optional chaining does not prevent destructuring on undefined.
To fix this, add a guard to ensure res and res.response are defined before
destructuring, or assert that _fetch always returns a valid response object.
Remove the lint-disable comment after applying the fix.
| // oxlint-disable-next-line no-unsafe-optional-chaining | ||
| const { data: roles, total_count } = res?.response as unknown as ClerkPaginatedResponse<RoleJSON>; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Repeated silencing of no-unsafe-optional-chaining hides potential crashes
Every paginated fetch follows the same pattern:
const { data, total_count } = res?.response as ClerkPaginatedResponse<...>;If res is undefined, destructuring still executes and crashes.
Instead of sprinkling // oxlint-disable, refactor once:
function safePaginated<R, T>(res: R | undefined): ClerkPaginatedResponse<T> {
if (!res || !(res as any).response) {
return { total_count: 0, data: [] };
}
return (res as any).response as ClerkPaginatedResponse<T>;
}Then:
const { data, total_count } = safePaginated<unknown, RoleJSON>(res);This removes all suppressions and centralises the guard.
Also applies to: 124-126, 153-156, 175-176, 198-199, 247-249
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/Organization.ts around lines 101 to 103,
the code unsafely destructures res?.response which can cause crashes if res is
undefined, and disables the linter repeatedly. To fix this, create a utility
function safePaginated that checks if res and res.response exist, returning a
default empty paginated response if not, otherwise returning res.response cast
properly. Replace all unsafe destructuring instances with calls to safePaginated
to centralize the guard and remove all no-unsafe-optional-chaining suppressions.
Apply this refactor also to lines 124-126, 153-156, 175-176, 198-199, and
247-249.
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (1)
51-59: 🛠️ Refactor suggestionRule-suppression hides a potential runtime crash
res?.response as unknown as ClerkPaginatedResponse<…>suppresses the linter, but ifBaseResource._fetchever resolves toundefined(non-200, network failure, etc.) the subsequent destructuring will throw at runtime.Consider defensive handling instead of disabling the lint rule:
- }).then(res => { - const { data: subscriptions, total_count } = - res?.response as unknown as ClerkPaginatedResponse<CommerceSubscriptionJSON>; // oxlint-disable-line no-unsafe-optional-chaining + }).then(res => { + if (!res?.response) { + return { total_count: 0, data: [] }; + } + const { data: subscriptions, total_count } = + res.response as ClerkPaginatedResponse<CommerceSubscriptionJSON>;Same pattern repeats for
getStatementsandgetPaymentAttempts.
Replacing doubleas unknown aswith a properly typed_fetch<T>()overload would eliminate the need for rule suppression altogether.Also applies to: 69-77, 88-92
♻️ Duplicate comments (12)
packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts (1)
48-50: Same unsafe pattern – please add a guard instead of disabling the ruleThe code is vulnerable to the same destructuring-from-
undefinedissue described inOrganizationMembership.ts. Apply a similar check to avoid a possible runtime crash.- const { data: paymentSources, total_count } = - res?.response as unknown as ClerkPaginatedResponse<CommercePaymentSourceJSON>; // oxlint-disable-line no-unsafe-optional-chaining + if (!res?.response) { + throw new Error('Empty response while fetching payment sources'); + } + const { data: paymentSources, total_count } = + res.response as ClerkPaginatedResponse<CommercePaymentSourceJSON>;packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (1)
32-34: Guard against empty response instead of suppressing lintReplicate the defensive check here; otherwise destructuring will explode if
responseis absent.- const { data: invites, total_count } = - res?.response as unknown as ClerkPaginatedResponse<UserOrganizationInvitationJSON>; // oxlint-disable-line no-unsafe-optional-chaining + if (!res?.response) { + throw new Error('Empty response while retrieving organization invitations'); + } + const { data: invites, total_count } = + res.response as ClerkPaginatedResponse<UserOrganizationInvitationJSON>;packages/clerk-js/src/core/resources/OrganizationSuggestion.ts (1)
34-36: Repeat of unsafe optional-chaining patternSame fix as previous comments – prefer explicit guard.
- const { data: suggestions, total_count } = - res?.response as unknown as ClerkPaginatedResponse<OrganizationSuggestionJSON>; // oxlint-disable-line no-unsafe-optional-chaining + if (!res?.response) { + throw new Error('Empty response while retrieving organization suggestions'); + } + const { data: suggestions, total_count } = + res.response as ClerkPaginatedResponse<OrganizationSuggestionJSON>;packages/clerk-js/src/core/resources/Organization.ts (5)
124-126: Same unsafe-destructuring issue as above – apply the guard pattern here as well.
153-155: Same unsafe-destructuring issue – needs guard.
175-176: Same unsafe-destructuring issue – needs guard.
198-199: Same unsafe-destructuring issue – needs guard.
247-248: Same unsafe-destructuring issue – needs guard.packages/react/package.json (1)
73-73: New OXLint script added Matches the pattern established in other packages.packages/dev-cli/package.json (1)
23-23: New OXLint script added Matches the pattern established in other packages.packages/expo-passkeys/package.json (1)
29-29: New OXLint script added Matches the pattern established in other packages.packages/nuxt/package.json (1)
64-64: New OXLint script added Matches the pattern established in other packages.
🧹 Nitpick comments (12)
packages/clerk-js/headless/index.js (1)
1-1: Verify OXlint directive validity.
The comment usesoxlint-disable-next-line; confirm that OXLint recognizes this syntax and that ESLint still enforces or ignores it as intended. If needed, mirror it with aneslint-disable-next-linefor consistent suppression across both linters.packages/clerk-js/no-rhc/index.js (1)
1-1: Ensure consistent linter disable directives.
This file usesoxlint-disable-next-lineto silence the TS import rule. Confirm OXLint support and consider adding a matchingeslint-disable-next-lineto maintain parallel behavior during the transition.packages/backend/src/runtime/node/crypto.js (1)
1-1: Align disable directives for both linters.
Here you’ve usedeslint-disable-next-linebut notoxlint-disable-next-line. To ensure OXLint also skips this rule, add the corresponding OXlint directive or confirm your configuration maps ESLint disables automatically.packages/clerk-js/svgTransform.js (1)
2-2: Standardize linter suppression comments.
You’ve added// oxlint-disable-line @typescript-eslint/no-require-importsalongside the existing ESLint suppression. Verify that both linters honor these, and consider documenting the pattern in CONTRIBUTING or your lint config to keep it uniform across packages.packages/clerk-js/src/utils/url.ts (1)
245-252: Duplicate null-byte check & regex may be simplified
/\0/is already covered by the broader/[\x00-\x1F]/control-character pattern below, so the extra entry (and the oxlint disable) is redundant. Removing it keeps the list compact and avoids maintaining two suppressions.🧰 Tools
🪛 Biome (1.9.4)
[error] 251-251: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 251-251: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
packages/localizations/package.json (1)
104-106: Verify script naming consistency across packagesMost packages now expose both
lintandlint:oxlint.
lint:attwsits between them here, while in several sibling packages it appears afterlint:oxlint.
Consistent ordering eases grep-ability and automated script harvesting; consider aligning the three scripts’ order across allpackage.jsons.turbo.json (1)
105-124: Potential double hashing & execution of the same glob setThe newly-added
linttask repeats almost the exactinputslist already present inlint:oxlint, yet its only job is to depend onlint:oxlint.
Turborepo will hash both tasks separately, doing the same glob walk twice.Options:
- Remove the
inputsarray fromlint(rely on the dependency’s cache), or- Delete
lintentirely and callturbo run lint:oxlintin CI.This avoids redundant hashing and shaves a few hundred ms from every run.
packages/astro/src/react/controlComponents.tsx (1)
88-109: Minor naming:unauthorized/authorizedVariables are now
unauthorizedandauthorizedbutauthorizedis never used besides an immediate return.
An inlinedchildren/fallbackwould remove two indirections and save a reallocation each render.
Low-impact, but worth tidying..oxlintrc.json (4)
7-11: Consider explicit environments for Node and BrowserCurrent
envsettings (builtin,es2021,commonjs) may not cover certain globals (e.g.,window,process). It may be safer to add"node": trueand/or"browser": truedepending on where files run.
12-40: Simplify and dedupeignorePatternsThere are overlapping patterns (e.g., both
.turboand**/.turbo/*) and very specific ignores that could be consolidated. Consider grouping related paths or using globs more sparingly to improve maintainability.
41-240: Leverage anextendspreset instead of inlining all rulesListing every rule explicitly makes the config hard to maintain. If OXLint provides a recommended preset, you could
extendsit and only override your project’s exceptions.
314-381: Refactor large integration-test overrideThis massive block of globals and rule tweaks could be extracted into a separate config (e.g.,
oxlint.integration.json) to keep the main file focused. It would also make onboarding and future updates easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
.oxlintrc.json(1 hunks)package.json(2 hunks)packages/agent-toolkit/package.json(1 hunks)packages/astro/package.json(1 hunks)packages/astro/src/astro-components/control/ProtectCSR.astro(1 hunks)packages/astro/src/react/controlComponents.tsx(4 hunks)packages/backend/package.json(1 hunks)packages/backend/src/runtime/node/crypto.js(1 hunks)packages/chrome-extension/package.json(1 hunks)packages/clerk-js/headless/index.js(1 hunks)packages/clerk-js/no-rhc/index.js(1 hunks)packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts(3 hunks)packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts(1 hunks)packages/clerk-js/src/core/resources/Organization.ts(6 hunks)packages/clerk-js/src/core/resources/OrganizationMembership.ts(1 hunks)packages/clerk-js/src/core/resources/OrganizationSuggestion.ts(1 hunks)packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts(1 hunks)packages/clerk-js/src/utils/url.ts(2 hunks)packages/clerk-js/svgTransform.js(1 hunks)packages/dev-cli/package.json(1 hunks)packages/elements/package.json(1 hunks)packages/expo-passkeys/package.json(1 hunks)packages/expo/package.json(1 hunks)packages/express/package.json(1 hunks)packages/fastify/package.json(1 hunks)packages/localizations/package.json(1 hunks)packages/nextjs/package.json(1 hunks)packages/nuxt/package.json(1 hunks)packages/react-router/package.json(1 hunks)packages/react/package.json(1 hunks)packages/remix/package.json(1 hunks)packages/shared/package.json(1 hunks)packages/shared/scripts/postinstall.mjs(1 hunks)packages/tanstack-react-start/package.json(1 hunks)packages/testing/package.json(1 hunks)packages/themes/package.json(1 hunks)packages/types/package.json(1 hunks)packages/upgrade/package.json(1 hunks)packages/vue/package.json(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
UserOrganizationInvitationJSON(483-493)
packages/clerk-js/src/core/resources/OrganizationMembership.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
OrganizationMembershipJSON(387-398)
packages/clerk-js/src/core/modules/commerce/payment-source-methods.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
CommercePaymentSourceJSON(636-646)
packages/clerk-js/src/core/resources/OrganizationSuggestion.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (1)
OrganizationSuggestionJSON(464-471)
packages/clerk-js/src/core/resources/Organization.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (4)
OrganizationMembershipRequestJSON(473-481)OrganizationMembershipJSON(387-398)OrganizationInvitationJSON(400-411)CommerceSubscriptionJSON(684-698)
packages/clerk-js/src/core/modules/commerce/CommerceBilling.ts (2)
packages/types/src/pagination.ts (1)
ClerkPaginatedResponse(21-30)packages/types/src/json.ts (2)
CommerceSubscriptionJSON(684-698)CommerceStatementJSON(655-662)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nextjs, chrome, 13)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
🔇 Additional comments (24)
packages/shared/scripts/postinstall.mjs (1)
52-52:✅ Verification successful
Confirm Node version compatibility with parameter-less catch.
Usingcatch {}without an error binding requires Node >= 14.6.0. Verify your minimum supported Node version in theenginesfield.
🏁 Script executed:
#!/bin/bash # Check the Node engine requirement in package.json jq -r '.engines.node // "not specified"' package.jsonLength of output: 65
No action needed: parameter-less catch is supported by Node ≥18.17.0
Yourpackage.jsonalready specifies
• engines.node: ">=18.17.0"
and Node 14.6.0+ added support forcatch {}syntax, so this usage is safe.package.json (1)
25-26: Lockoxlintto an explicitly tested versionIntroducing
oxlintis great 🎉. However, depending on^1.0.0means that any future 1.x release (1.1, 1.2, …) will be installed automatically and may introduce new rules or breaking changes that could suddenly fail CI.- "oxlint": "^1.0.0", + "oxlint": "1.0.0",Pinning to the exact version keeps the toolchain deterministic; bump intentionally once the new version has been vetted.
Also applies to: 124-125
packages/shared/package.json (1)
137-138: Script addition looks good
"lint:oxlint": "oxlint --quiet src"mirrors the existing ESLint script and scopes linting to the package’s source – sensible default.packages/nextjs/package.json (1)
69-70: Next.js package OXLint integration – OKConsistent with other packages; no further comments.
packages/remix/package.json (1)
72-72: Addlint:oxlintscript
Introduces OXLint alongside ESLint for faster linting of thesrcdirectory, matching the repo convention.packages/elements/package.json (1)
68-68: Addlint:oxlintscript
Aligns with other packages by enabling OXLint onsrcin quiet mode for consistent linting.packages/backend/package.json (1)
98-98: Addlint:oxlintscript
Enables quiet OXLint runs for backend sources, in line with the monorepo’s updated lint pipeline.packages/types/package.json (1)
39-39: Addlint:oxlintscript
Introduces OXLint for faster static checks in the types package, consistent with other workspace packages.packages/themes/package.json (1)
38-39: Addlint:oxlintscript and update comma
The comma afterlint:attwis correctly added to maintain JSON validity, andlint:oxlintis introduced for standard OXLint usage.packages/tanstack-react-start/package.json (1)
64-64: Add OXLint lint script
The newlint:oxlintscript follows the established pattern and integrates Oxlint into this package’s workflow.packages/react-router/package.json (1)
77-77: Add OXLint lint script
Thelint:oxlintentry is correctly positioned afterlint:attwand beforelint:publint, matching other packages.packages/clerk-js/package.json (1)
49-49: Add OXLint lint script
This update aligns the Clerk JS package with the new Oxlint setup across the monorepo.packages/agent-toolkit/package.json (1)
44-44: Add OXLint lint script
The new script integrates Oxlint into the agent-toolkit package, consistent with the other packages.packages/astro/package.json (1)
85-85: Add OXLint lint script
Includingenv.d.tsin the Oxlint invocation mirrors the existing ESLint configuration—good catch.packages/chrome-extension/package.json (1)
43-43:✅ Verification successful
New OXLint script added The
lint:oxlintcommand is correctly integrated alongside ESLint. Ensure thatoxlintis declared in the rootdevDependenciesand properly hoisted into this workspace.Run:
🏁 Script executed:
#!/bin/bash # Verify that oxlint is declared in the root package.json grep -R '"oxlint"' -n package.jsonLength of output: 99
Approve: OXLint script and dependency are correctly configured
- Verified that the root package.json declares
"oxlint": "^1.0.0"in devDependencies (line 124), ensuring it’s hoisted into this workspace.No further action required.
packages/expo/package.json (1)
75-75: Introducelint:oxlintscript
The newlint:oxlintscript aligns with the monorepo’s linting workflow. Ensure thatoxlintis added as a devDependency at the repository root and that a.oxlintrc.jsonconfig exists to standardize rules.packages/vue/package.json (1)
53-53: Addlint:oxlintscript
Consistently adds OXLint for the Vue package’ssrcdirectory.packages/fastify/package.json (1)
60-60: Integrate OXLint script
Addslint:oxlintto run OXLint onsrcin the Fastify package.packages/express/package.json (1)
62-62: Add OXLint to Express package
Thelint:oxlintscript is correctly added to lint thesrcdirectory quietly.packages/upgrade/package.json (1)
23-23: Introducelint:oxlintscript
Adds OXLint linting support for the upgrade package’s source files.packages/astro/src/react/controlComponents.tsx (1)
62-70: Unnecessary fragment removal is fine — but mind whitespaceRemoving the fragment means literal whitespace/newlines around
{children}become text nodes.
React usually collapses them, yet in tight layout components this can matter.
Double-check visual diffs for any spacing regressions..oxlintrc.json (3)
4-6: Verify disabling of thecorrectnesscategoryYou’ve globally turned off the entire
correctnesscategory, which may silence critical rule violations. Ensure this was intentional and that you’re still catching all needed error-prone patterns.
199-218: Verify theno-restricted-importspattern hackThe entry using
"group": ["!@clerk/shared/*"]is a non-standard workaround. Please confirm that OXLint’s import plugin supports negated patterns in this shape and that it behaves as expected.
278-283: Clarify the ES version override for TypeScript filesYou’ve set
es2018specifically for all TS/CTS/TSX files even though the global env ises2021. This downgrade may disable syntax/features you intend to lint. Ensure this is by design.
| const { data: suggestions, total_count } = | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; | ||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; // oxlint-disable-line no-unsafe-optional-chaining | ||
|
|
There was a problem hiding this comment.
Potential runtime crash – destructuring from undefined is still possible
Disabling the lint rule masks the underlying danger: if _fetch ever resolves to undefined (or to an object without response), res?.response is undefined and destructuring will throw at runtime.
Prefer an explicit guard or non-null assertion rather than silencing the warning.
- const { data: suggestions, total_count } =
- res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; // oxlint-disable-line no-unsafe-optional-chaining
+ if (!res?.response) {
+ throw new Error('Empty response while retrieving organization memberships');
+ }
+ const { data: suggestions, total_count } =
+ res.response as ClerkPaginatedResponse<OrganizationMembershipJSON>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: suggestions, total_count } = | |
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; | |
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; // oxlint-disable-line no-unsafe-optional-chaining | |
| if (!res?.response) { | |
| throw new Error('Empty response while retrieving organization memberships'); | |
| } | |
| const { data: suggestions, total_count } = | |
| res.response as ClerkPaginatedResponse<OrganizationMembershipJSON>; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/OrganizationMembership.ts around lines
42 to 44, the destructuring of `res?.response` can cause a runtime crash if
`res` or `res.response` is undefined. To fix this, add an explicit check to
ensure `res` and `res.response` are defined before destructuring, or use a
non-null assertion if you are certain the value is never null. Remove the lint
disable comment and handle the potential undefined case safely to prevent
runtime errors.
| // oxlint-disable-next-line no-unsafe-optional-chaining | ||
| const { data: roles, total_count } = res?.response as unknown as ClerkPaginatedResponse<RoleJSON>; | ||
|
|
There was a problem hiding this comment.
Do not paper over no-unsafe-optional-chaining; harden the fetch logic
If res or res.response is missing, the destructuring will crash despite the cast.
Add a guard (or handle the error) instead of disabling the lint rule.
- // oxlint-disable-next-line no-unsafe-optional-chaining
- const { data: roles, total_count } = res?.response as unknown as ClerkPaginatedResponse<RoleJSON>;
+ if (!res?.response) {
+ throw new Error('Empty response while retrieving roles');
+ }
+ const { data: roles, total_count } = res.response as ClerkPaginatedResponse<RoleJSON>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // oxlint-disable-next-line no-unsafe-optional-chaining | |
| const { data: roles, total_count } = res?.response as unknown as ClerkPaginatedResponse<RoleJSON>; | |
| if (!res?.response) { | |
| throw new Error('Empty response while retrieving roles'); | |
| } | |
| const { data: roles, total_count } = res.response as ClerkPaginatedResponse<RoleJSON>; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/resources/Organization.ts around lines 101 to 103,
the code disables the no-unsafe-optional-chaining lint rule and destructures
res?.response without checking if res or res.response exists, which can cause a
runtime crash. Fix this by adding a guard to verify that res and res.response
are defined before destructuring. If either is missing, handle the error or
return early to avoid unsafe access instead of casting and disabling the lint
rule.
| // Check for null bytes in pathname | ||
| if (url.pathname.includes('\0')) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Encoded %00 slips through the new null-byte guard
url.pathname.includes('\0') only matches the literal null character, but URL.pathname keeps percent-encodings (%00). An attacker could therefore bypass this check with /foo%00bar.
- if (url.pathname.includes('\0')) {
+ const decodedPath = decodeURIComponent(url.pathname);
+ if (decodedPath.includes('\0')) {
return true;
}Optionally, drop the dedicated check and rely on /[\x00-\x1F]/ after decoding to catch all control characters in one place.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for null bytes in pathname | |
| if (url.pathname.includes('\0')) { | |
| return true; | |
| } | |
| // Check for null bytes in pathname | |
| const decodedPath = decodeURIComponent(url.pathname); | |
| if (decodedPath.includes('\0')) { | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In packages/clerk-js/src/utils/url.ts around lines 263 to 266, the current check
for null bytes using url.pathname.includes('\0') misses encoded null bytes like
%00 because URL.pathname retains percent-encodings. To fix this, decode the
pathname before checking and replace the dedicated null byte check with a single
regex that detects all control characters (e.g., /[\x00-\x1F]/) on the decoded
pathname to catch both literal and encoded control characters.
packages/testing/package.json
Outdated
| "lint": "eslint src", | ||
| "lint:oxlint": "oxlint --quiet src" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add oxlint to devDependencies for local script execution
"lint:oxlint": "oxlint --quiet src" assumes oxlint is available either globally or at the repo root.
Adding the binary to this package’s devDependencies (or using the workspace-root script via pnpm exec) avoids “command not found” issues when the package is worked on in isolation.
"devDependencies": {
+ "oxlint": "^1.0.0",
"@playwright/test": "^1.44.0",
"cypress": "^13.9.0"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lint": "eslint src", | |
| "lint:oxlint": "oxlint --quiet src" | |
| }, | |
| "devDependencies": { | |
| "oxlint": "^1.0.0", | |
| "@playwright/test": "^1.44.0", | |
| "cypress": "^13.9.0" | |
| }, |
🤖 Prompt for AI Agents
In packages/testing/package.json around lines 72 to 74, the "lint:oxlint" script
uses the "oxlint" command without ensuring it is installed locally, which can
cause "command not found" errors when working in isolation. Add "oxlint" to the
devDependencies of this package to guarantee the binary is available locally, or
alternatively modify the script to run "pnpm exec oxlint --quiet src" to use the
workspace-root version safely.
| // oxlint-disable-next-line no-unused-expressions | ||
| isUnauthorized ? this.defaultSlot.setAttribute('hidden', '') : this.defaultSlot.removeAttribute('hidden'); | ||
| } | ||
|
|
||
| if (this.fallbackSlot) { | ||
| // oxlint-disable-next-line no-unused-expressions | ||
| isUnauthorized ? this.fallbackSlot.removeAttribute('hidden') : this.fallbackSlot.setAttribute('hidden', ''); | ||
| } |
There was a problem hiding this comment.
Replace ternary side-effects with explicit if to drop the disable comment
Disabling no-unused-expressions hides a genuine readability smell.
Using an explicit if keeps the intent clear and lets the code pass both ESLint and Oxlint without suppression.
-// oxlint-disable-next-line no-unused-expressions
-isUnauthorized ? this.defaultSlot.setAttribute('hidden', '') : this.defaultSlot.removeAttribute('hidden');
+if (isUnauthorized) {
+ this.defaultSlot.setAttribute('hidden', '');
+} else {
+ this.defaultSlot.removeAttribute('hidden');
+}
...
-// oxlint-disable-next-line no-unused-expressions
-isUnauthorized ? this.fallbackSlot.removeAttribute('hidden') : this.fallbackSlot.setAttribute('hidden', '');
+if (isUnauthorized) {
+ this.fallbackSlot.removeAttribute('hidden');
+} else {
+ this.fallbackSlot.setAttribute('hidden', '');
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // oxlint-disable-next-line no-unused-expressions | |
| isUnauthorized ? this.defaultSlot.setAttribute('hidden', '') : this.defaultSlot.removeAttribute('hidden'); | |
| } | |
| if (this.fallbackSlot) { | |
| // oxlint-disable-next-line no-unused-expressions | |
| isUnauthorized ? this.fallbackSlot.removeAttribute('hidden') : this.fallbackSlot.setAttribute('hidden', ''); | |
| } | |
| if (this.defaultSlot) { | |
| if (isUnauthorized) { | |
| this.defaultSlot.setAttribute('hidden', ''); | |
| } else { | |
| this.defaultSlot.removeAttribute('hidden'); | |
| } | |
| } | |
| if (this.fallbackSlot) { | |
| if (isUnauthorized) { | |
| this.fallbackSlot.removeAttribute('hidden'); | |
| } else { | |
| this.fallbackSlot.setAttribute('hidden', ''); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/astro/src/astro-components/control/ProtectCSR.astro around lines 47
to 54, replace the ternary expressions used solely for side effects with
explicit if-else statements. This means using clear if conditions to call
setAttribute or removeAttribute instead of the current ternary operators, which
improves readability and removes the need for disabling the
no-unused-expressions lint rule.
| export const ClerkLoaded = ({ children }: React.PropsWithChildren): React.ReactNode => { | ||
| const isLoaded = useSafeIsLoaded(); | ||
|
|
||
| if (!isLoaded) { | ||
| return null; | ||
| } | ||
|
|
||
| return <>{children}</>; | ||
| return children; | ||
| }; |
There was a problem hiding this comment.
Return type should be React.ReactElement | null, not React.ReactNode
A React component must return a React element (or null).
Allowing any ReactNode widens the type and enables returning primitives (string, number) accidentally, which React will render but breaks wrapper utilities (cloneElement, isValidElement, etc.).
-export const ClerkLoaded = ({ children }: React.PropsWithChildren): React.ReactNode => {
+export const ClerkLoaded = ({ children }: React.PropsWithChildren): React.ReactElement | null => {Apply the same change to ClerkLoading and Protect.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const ClerkLoaded = ({ children }: React.PropsWithChildren): React.ReactNode => { | |
| const isLoaded = useSafeIsLoaded(); | |
| if (!isLoaded) { | |
| return null; | |
| } | |
| return <>{children}</>; | |
| return children; | |
| }; | |
| export const ClerkLoaded = ({ children }: React.PropsWithChildren): React.ReactElement | null => { | |
| const isLoaded = useSafeIsLoaded(); | |
| if (!isLoaded) { | |
| return null; | |
| } | |
| return children; | |
| }; |
🤖 Prompt for AI Agents
In packages/astro/src/react/controlComponents.tsx around lines 52 to 60, change
the return type of the ClerkLoaded component from React.ReactNode to
React.ReactElement | null to ensure it only returns a valid React element or
null. Also, apply this same return type change to the ClerkLoading and Protect
components to prevent returning primitives and maintain compatibility with React
utilities like cloneElement and isValidElement.
| @@ -137,7 +142,7 @@ | |||
| ({ clerk, ...handleRedirectCallbackParams }: WithClerkProp<HandleOAuthCallbackParams>) => { | |||
| React.useEffect(() => { | |||
| void clerk?.handleRedirectCallback(handleRedirectCallbackParams); | |||
| }, []); | |||
| }, []); // eslint-disable-line react-hooks/exhaustive-deps | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid blanket disable of react-hooks/exhaustive-deps
Instead of disabling the rule, list the actual stable deps (clerk, those in handleRedirectCallbackParams).
If they’re intentionally omitted, add a brief comment why re-invocation is undesired; this helps future maintainers.
-}, []); // eslint-disable-line react-hooks/exhaustive-deps
+// `clerk` is stable (ClerkProvider guarantees identity) and callback params are
+// static at mount, so we run once intentionally.
+}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, []); // eslint-disable-line react-hooks/exhaustive-deps | |
| // `clerk` is stable (ClerkProvider guarantees identity) and callback params are | |
| // static at mount, so we run once intentionally. | |
| }, []); |
🤖 Prompt for AI Agents
In packages/astro/src/react/controlComponents.tsx at line 145, avoid disabling
the react-hooks/exhaustive-deps rule with a blanket comment. Instead, explicitly
list all stable dependencies such as `clerk` and any variables used inside
`handleRedirectCallbackParams` in the dependency array. If some dependencies are
intentionally omitted to prevent unwanted re-invocation, add a clear comment
explaining this decision for future maintainers.
| @@ -0,0 +1,431 @@ | |||
| { | |||
| "$schema": "./node_modules/oxlint/configuration_schema.json", | |||
| "plugins": ["typescript", "import", "jsx-a11y", "react", "unicorn"], | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Critical: Missing react-hooks plugin
You’ve enabled react-hooks/rules-of-hooks and react-hooks/exhaustive-deps without declaring the react-hooks plugin. Those rules won’t be applied unless you add "react-hooks" to your plugins list.
🤖 Prompt for AI Agents
In .oxlintrc.json at line 3, the "react-hooks" plugin is missing from the
plugins array, which prevents the react-hooks rules from being applied. Add
"react-hooks" to the plugins list alongside the existing plugins to enable those
rules properly.
| @@ -40,7 +40,7 @@ export class OrganizationMembership extends BaseResource implements Organization | |||
| }).then(res => { | |||
| // TODO: Fix typing | |||
| const { data: suggestions, total_count } = | |||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; | |||
| res?.response as unknown as ClerkPaginatedResponse<OrganizationMembershipJSON>; // oxlint-disable-line no-unsafe-optional-chaining | |||
There was a problem hiding this comment.
should we maybe make this rule a warning (perhaps specifically for clerk-js) instead of silencing it?
There was a problem hiding this comment.
@dstaley The reason I took this approach is because our warnings have been piling up. Setting this rule to warn in general would, IMHO, likely have a greater chance of this error being introduced in future code vs outright failing.
| @@ -102,6 +102,27 @@ | |||
| "cache": false | |||
| }, | |||
| "lint": { | |||
| "dependsOn": ["lint:oxlint"], | |||
There was a problem hiding this comment.
the approach we took for vitest in clerk-js was to update the lint task to run both jest and vitest. not saying we should do it that way here, just offering it in the event that sounds better than creating a new turbo task
There was a problem hiding this comment.
I kinda like that we can cache these individually and, I feel, that it enforces a generic interface (turbo) with which we run commands.
Description
This PR introduces Oxlint 1.0 as Oxlint is unbelievably faster than ESLint:
Commands:
turbo lint --filter=\!@clerk/localizations --force --onlyturbo lint:oxlint --filter=\!@clerk/localizations --force --only(NOTE:
@clerk/localizationswouldn't complete with ESLint.)PR Notes:
linttask now depends onlint:oxlintso, if there's an error, it fails fast.eslint-plugin-oxlint.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Chores