SK-2528: update jwt-decode library for Node SDK#282
SK-2528: update jwt-decode library for Node SDK#282skyflow-himanshu wants to merge 1 commit intomainfrom
Conversation
|
✅ Gitleaks Findings: No secrets detected. Safe to proceed! |
|
Semgrep Findings: Issues with Error level severity are found (Error is Highest severity in Semgrep), Please resolve the issues before merging. |
There was a problem hiding this comment.
Pull request overview
Upgrades the Node SDK’s JWT decoding dependency to jwt-decode@4 and updates internal/test imports accordingly.
Changes:
- Bumped
jwt-decodefrom^2.2.0to^4.0.0 - Switched code and tests from default import (
jwt_decode) to named import (jwtDecode) - Updated jwt utility implementation to use the new import
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| test/vault/utils/utils.test.js | Updates test imports and mocks to align with jwtDecode named export. |
| test/vault/utils/jwt-utils/jwt.test.js | Updates jwt-utils tests to use jwtDecode named export and related mocks. |
| src/utils/jwt-utils/index.ts | Updates implementation to call jwtDecode instead of default import. |
| package.json | Bumps jwt-decode dependency to ^4.0.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "js-base64": "3.7.7", | ||
| "jsonwebtoken": "^9.0.3", | ||
| "jwt-decode": "^2.2.0", | ||
| "jwt-decode": "^4.0.0", |
There was a problem hiding this comment.
jwt-decode@4 is ESM-only. If this Node SDK is published/compiled to CommonJS (common for TS SDKs without \"type\": \"module\"), consumers will hit runtime errors when requiring the SDK (e.g., ERR_REQUIRE_ESM). If the SDK isn't ESM end-to-end, consider keeping jwt-decode on a CJS-compatible major (or switching the SDK build/publish output to ESM) before merging this dependency bump.
| "jwt-decode": "^4.0.0", | |
| "jwt-decode": "^3.1.2", |
| import { jwtDecode } from 'jwt-decode'; | ||
| import { isTokenValid, isExpired } from '../../../../src/utils/jwt-utils'; | ||
|
|
||
| jest.mock('jwt-decode'); |
There was a problem hiding this comment.
With a named export (jwtDecode), jest.mock('jwt-decode') may not produce a mocked jwtDecode function in the way this test expects—especially since jwt-decode@4 is ESM-only. This can cause jwtDecode.mockReturnValue / mockImplementation to throw because jwtDecode is not a Jest mock. Prefer providing an explicit Jest mock factory that exports jwtDecode as jest.fn() (and sets __esModule: true when applicable), or use Jest’s ESM mocking APIs if the test environment is ESM.
| jest.mock('jwt-decode'); | |
| jest.mock('jwt-decode', () => ({ | |
| __esModule: true, | |
| jwtDecode: jest.fn(), | |
| })); |
|
|
||
| beforeEach(() => { | ||
| jwt_decode.mockReturnValue(mockDecodedPayload); | ||
| jwtDecode.mockReturnValue(mockDecodedPayload); |
There was a problem hiding this comment.
With a named export (jwtDecode), jest.mock('jwt-decode') may not produce a mocked jwtDecode function in the way this test expects—especially since jwt-decode@4 is ESM-only. This can cause jwtDecode.mockReturnValue / mockImplementation to throw because jwtDecode is not a Jest mock. Prefer providing an explicit Jest mock factory that exports jwtDecode as jest.fn() (and sets __esModule: true when applicable), or use Jest’s ESM mocking APIs if the test environment is ESM.
|
|
||
| test('should return false in catch', () => { | ||
| jwt_decode.mockImplementation(() => { | ||
| jwtDecode.mockImplementation(() => { |
There was a problem hiding this comment.
With a named export (jwtDecode), jest.mock('jwt-decode') may not produce a mocked jwtDecode function in the way this test expects—especially since jwt-decode@4 is ESM-only. This can cause jwtDecode.mockReturnValue / mockImplementation to throw because jwtDecode is not a Jest mock. Prefer providing an explicit Jest mock factory that exports jwtDecode as jest.fn() (and sets __esModule: true when applicable), or use Jest’s ESM mocking APIs if the test environment is ESM.
|
|
||
| test('should return false in catch for isExpired', () => { | ||
| jwt_decode.mockImplementation(() => { | ||
| jwtDecode.mockImplementation(() => { |
There was a problem hiding this comment.
With a named export (jwtDecode), jest.mock('jwt-decode') may not produce a mocked jwtDecode function in the way this test expects—especially since jwt-decode@4 is ESM-only. This can cause jwtDecode.mockReturnValue / mockImplementation to throw because jwtDecode is not a Jest mock. Prefer providing an explicit Jest mock factory that exports jwtDecode as jest.fn() (and sets __esModule: true when applicable), or use Jest’s ESM mocking APIs if the test environment is ESM.
| } | ||
| let isJwtExpired = false; | ||
| const decoded: JwtPayload = jwt_decode(token); | ||
| const decoded: JwtPayload = jwtDecode(token); |
There was a problem hiding this comment.
Consider using the generic type parameter on jwtDecode (e.g., jwtDecode<JwtPayload>(token)) instead of annotating the variable as JwtPayload. This keeps the type aligned with the function’s return type and avoids masking type incompatibilities if the library typings change.
| if (token === "") return false | ||
| let isJwtExpired = false; | ||
| const decoded: JwtPayload = jwt_decode(token); | ||
| const decoded: JwtPayload = jwtDecode(token); |
There was a problem hiding this comment.
Consider using the generic type parameter on jwtDecode (e.g., jwtDecode<JwtPayload>(token)) instead of annotating the variable as JwtPayload. This keeps the type aligned with the function’s return type and avoids masking type incompatibilities if the library typings change.
2 participants
Why:
Update libraries for Node SDK
Goal:
Upgrade jwt-decode to v4.0.0.
Attempt upgrade of @types/node to v25.2.0 and node-fetch to v3.3.0 and identify compatibility issues with generated files. so we are keeping the versions unchanged for now.