-
Notifications
You must be signed in to change notification settings - Fork 13
bump Jose to 6 to support Cloudflare workers, update ESlint #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce a new ESLint configuration file with detailed rules for JavaScript and TypeScript files, including plugin integrations and formatting enforcement. Several files received minor formatting improvements, such as adjusting line breaks, indentation, and import order, without altering logic or behavior. One test file updates an import statement for clarity. Additionally, a TypeScript interface was updated to change the return type of a method from Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant ESLint
participant Prettier
participant TypeScript ESLint Plugin
Developer->>ESLint: Run lint on project
ESLint->>TypeScript ESLint Plugin: Lint TypeScript files with custom rules
ESLint->>Prettier: Apply formatting overrides
ESLint-->>Developer: Report linting and formatting issues
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.eslintrc.yml
is excluded by!**/*.yml
package.json
is excluded by!**/*.json
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
,!**/*.yaml
📒 Files selected for processing (7)
eslint.config.mjs
(1 hunks)lib/__tests__/sdk/oauth2-flows/AuthorizationCode.spec.ts
(1 hunks)lib/__tests__/sdk/oauth2-flows/ClientCredentials.spec.ts
(1 hunks)lib/sdk/clients/server/index.ts
(1 hunks)lib/sdk/clients/server/with-auth-utilities.ts
(1 hunks)lib/sdk/exceptions.ts
(1 hunks)lib/sdk/utilities/types.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lib/sdk/clients/server/index.ts (1)
lib/sdk/clients/types.ts (5)
ACClientOptions
(17-19)CCClientOptions
(16-16)PKCEClientOptions
(15-15)ACClient
(21-21)CCClient
(22-22)
🔇 Additional comments (10)
lib/sdk/clients/server/with-auth-utilities.ts (1)
5-8
: Reordering of imported types looks good.This change simply reorders the imports from
../../utilities/index.js
to alphabetically listFlagType
beforeGetFlagType
. This is a minor formatting change that doesn't affect functionality and likely aligns with the new ESLint configuration.lib/sdk/exceptions.ts (1)
7-10
: Constructor formatting improvement looks good.The constructor parameters for
KindeSDKError
have been reformatted to use multiple lines rather than a single line. This improves readability and is consistent with the formatting conventions established in the new ESLint configuration.eslint.config.mjs (1)
1-67
: New ESLint configuration looks comprehensive and well-structured.This new configuration file properly sets up ESLint with modern best practices:
- Uses the new flat config format with
defineConfig
- Ignores build directories
- Sets appropriate TypeScript rules
- Integrates with Prettier
- Configures proper plugin support
The configuration effectively disables some strict TypeScript rules that might be too restrictive for this codebase while maintaining good standards overall.
lib/__tests__/sdk/oauth2-flows/ClientCredentials.spec.ts (2)
38-38
: Key handling matches Jose v6 signature requirements.The usage of
importJWK
appears compatible with Jose v6. This method is used to import JWK for validation, which is a critical part of the authorization flow. The change fromcreateLocalJWKSet
to just usingimportJWK
directly is consistent with Jose v6's API.
1-1
:✅ Verification successful
Import statement cleanup for Jose v6 compatibility.
The removal of
createLocalJWKSet
from the import statement is likely part of the Jose library upgrade from v5 to v6. This change aligns with the PR objective to support Cloudflare workers.
🏁 Script executed:
#!/bin/bash # Check how createLocalJWKSet import was used previously and if it's still needed echo "Checking if createLocalJWKSet was used in this file previously:" git show HEAD~1:lib/__tests__/sdk/oauth2-flows/ClientCredentials.spec.ts | grep -A 3 "import.*jose" echo "Checking if this change is consistent across other test files:" fd --type f "*.spec.ts" lib/__tests__ | xargs grep -l "createLocalJWKSet" || echo "No other files import createLocalJWKSet"Length of output: 1049
🏁 Script executed:
#!/bin/bash # Correctly search for createLocalJWKSet across all spec.ts files in tests fd --type f --glob '*.spec.ts' lib/__tests__ | xargs grep -l "createLocalJWKSet" || echo "No other files import createLocalJWKSet"Length of output: 169
Import statement cleanup verified for Jose v6 compatibility
The
createLocalJWKSet
import was removed fromlib/__tests__/sdk/oauth2-flows/ClientCredentials.spec.ts
and no other test files reference it. This aligns with the upgrade to Jose v6 and Cloudflare Workers support—no further changes needed.lib/__tests__/sdk/oauth2-flows/AuthorizationCode.spec.ts (1)
384-384
: Clean formatting improvementThis change simplifies the code by reformatting a multi-line expression to a single line, which improves readability while maintaining the same functionality.
lib/sdk/clients/server/index.ts (2)
17-20
: Improved readability with better indentationThe indentation changes in the nested ternary expressions make the code structure clearer and more consistent, improving readability without changing functionality.
24-27
: Consistent indentation pattern appliedThis indentation change follows the same pattern applied earlier, maintaining a consistent style throughout the conditional type definitions.
lib/sdk/utilities/types.ts (2)
1-1
: Updated jose import for Cloudflare Workers compatibilityChanged from importing
KeyLike
toCryptoKey
from the jose package, which aligns with the PR objective to upgrade Jose to version 6 for Cloudflare Workers compatibility.
49-49
:✅ Verification successful
Updated type signature for jose v6 compatibility
The return type of
keyProvider
has been updated fromKeyLike
toCryptoKey
to maintain consistency with the new jose v6 API. This change is necessary for proper functioning with Cloudflare Workers.To verify this change is correctly implemented across the codebase:
🏁 Script executed:
#!/bin/bash # Check if there are any remaining references to KeyLike that should have been updated rg "KeyLike" --type tsLength of output: 22
🏁 Script executed:
#!/bin/bash # Double-check for any remaining references to KeyLike across the entire repository rg "KeyLike"Length of output: 12
No remaining
KeyLike
references;keyProvider
signature is consistentI’ve verified across the repository that there are no remaining occurrences of
KeyLike
. The update to returnPromise<CryptoKey | Uint8Array>
aligns with the jose v6 API and is correctly implemented.
parserOptions: { | ||
project: './tsconfig.json', | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify TypeScript project configuration.
The parser options reference ./tsconfig.json
directly. Ensure this path is correct relative to where ESLint will be executed, especially if your build or test scripts run from different directories.
🏁 Script executed:
#!/bin/bash
# Check if tsconfig.json exists in the expected location
if [ -f "./tsconfig.json" ]; then
echo "tsconfig.json found in root directory"
else
echo "tsconfig.json not found in root directory"
echo "Searching for tsconfig.json in the repository:"
find . -name "tsconfig.json" | sort
fi
Length of output: 260
Missing TypeScript Configuration for ESLint Parser
It appears there’s no tsconfig.json
at the repository root (or anywhere else), yet your ESLint config is pointing to ./tsconfig.json
. ESLint will error out when it can’t find that file. Please update as follows:
• Either add (or move) your tsconfig.json
to the project root
• Or correct the path in eslint.config.mjs
(lines 34–36) under parserOptions.project
to point at the actual TS config location
Explain your changes
Jose 5 do not work correctly with Kinde on Cloudflare workers. 6 fixes those issues.
I updated ESLint to 9. Feel free to adjust the config etc but it now works, so take it as good starting point.
Part of tests is failing due to some probably silly issue around ESM mocks.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.