⭐️ [Refactor] [bun > pnpm] Feature/jss 93 refactor bun pnpm #889

Ansonhkg · 2025-09-15T14:12:29Z

WHAT

A refactor from Bun to PNPM
Migrated NX from 17.3.0 and pinned to 21.2.1
Upgraded esbuild to 0.19.2 to satisfy Nx, removed typedoc-theme-hierarchy because it blocked upgrades, and moved typedoc to 0.28.12 for compatibility with TypeScript 5.8+.
Use dotenvx to load .env and run the gen:local-network-context using tsx. Previously Bun would automatically detect and load the .env file.
Migrated test to jest

Do not merge this until these are merged

⭐️ [BREAKING/Feature] Naga add hardcoded keysets 3 #880

Based off this branch (pnpm migration)

UPDATE 22/09/2025

Do not merge until load testing is done or ✅ from @hwrdtm

…typedoc-theme-hierarchy because it blocked upgrades, and moved typedoc to 0.28.12 for compatibility with TypeScript 5.8+.

… minimal module mapping, dotenvx for envs - Switched Bun TS runner to Node + tsx with dotenvx wrapping for env loading. - Replaced Bun E2E runner with Jest using a dedicated jest e2e config - Resolved CJS/ESM breakages by using Babel transfomer for .ts|.js| in Jest. - Avoided duplicated module resolution: ignored `.nx/` and `dist/` in Jest. - Prevented hijacking external `@lit-protocol/*` and only map local packages

… only supports `babel-jest@^.29`.

…update reset script in package.json

- ESlint 8.48.0 no longer supported - @walletconnect/ethereum-provider is unused - uuidv4 to `crypto.randomUUID()` `@simplewebauthn/typescript-types` is NOT yet replaced with later types as unsure with side effects, locked-in to a particular version instead.

…s and remove module type; add TypeScript configuration

- remove unused artillery scripts - and update test command in package.json - remove unused packages

- Update type definitions to replace deprecated PKPInfo with PKPData across auth and e2e packages. - Modify Jest configuration to match all spec files in e2e tests. - Add .e2e to .gitignore for better file management. - Enhance e2e package structure with new types and utility functions for improved clarity and maintainability.

…ata. This changes are added in #891 and #892 but they are pull in so this PR can build

- moved inline import to top of the file - migrated to jest syntax - export utils from e2e and consumed by artillery - moved artillery as its own package - migrated script commands

…ready

…or better type safety

glitch003

seems pretty straightforward - looks good to me

…tocol/js-sdk into feature/jss-107-refactor-move-e2e-and-artillery-into-packages

…ve-e2e-and-artillery-into-packages Feature/jss 107 refactor move e2e and artillery into packages

MaximusHaximus

👍 Nothing blocking, added a few nits and it looks like we've still got auth data schemas duplicated in a few places which I noted.

MaximusHaximus · 2025-09-23T11:43:09Z

packages/artillery/src/init.ts

+import { getOrCreatePkp } from '@lit-protocol/e2e/src/helper/pkp-utils';
+import * as NetworkManager from '@lit-protocol/e2e/src/helper/NetworkManager';


Should this be importing from the package exports instead of reaching into the sub-director?

yes it should!

MaximusHaximus · 2025-09-23T11:51:38Z

...gers/LitChainClient/apis/highLevelApis/PKPPermissionsManager/handlers/getPKPsByAuthMethod.ts

+    metadata: z.unknown().optional(),
+  })
+  .superRefine((val, ctx) => {
+    if (val.authMethodType == null) {


Why not just make this required on the schema?

I think we need to update this branch to use the changes we made in the WebAuthn bug fix branch and reuse the auth data schema changes we made there.

MaximusHaximus · 2025-09-23T11:52:02Z

...gers/LitChainClient/apis/highLevelApis/PKPPermissionsManager/handlers/getPKPsByAuthMethod.ts

+        message: 'authMethodType is required',
+      });
+    }
+    if (val.authMethodId == null) {


Why not just make this required on the schema, so that it's type-safe for consumers as well as being enforced automatically by ZOD's parsing?

same as #889 (comment)

MaximusHaximus · 2025-09-23T11:52:59Z

...gers/LitChainClient/apis/highLevelApis/PKPPermissionsManager/handlers/getPKPsByAuthMethod.ts

+      });
+    }
+  })
+  .transform(({ authMethodType, authMethodId, accessToken }) => ({


Why not use z.coerce.bigint()?

same as #889 (comment)

MaximusHaximus · 2025-09-23T11:53:15Z

...gers/LitChainClient/apis/highLevelApis/PKPPermissionsManager/handlers/getPKPsByAuthMethod.ts

+  })
+  .transform(({ authMethodType, authMethodId, accessToken }) => ({
+    authMethodType:
+      typeof authMethodType === 'bigint'


Why not use z.coerce.bigint()?

same as #889 (comment)

MaximusHaximus · 2025-09-23T12:45:20Z

packages/e2e/src/e2e.spec.ts

+          (effChain.rpcUrls as any)['public']?.http
+        );
+      }
+    } catch {}


Why ignore errors here? If this code doesn't work, shouldn't this suite fail?

Yea you're right - I didn’t want the tests to fail since they’re purely for logging purposes to check the overridden private RPC URL, but we should just throw an error.

MaximusHaximus · 2025-09-23T12:46:48Z

packages/e2e/src/helper/tests/pkp-sign.ts

+      // signingScheme: 'EcdsaK256Sha256',
      authContext: getAuthContext(),
-      pubKey: ctx.aliceViemAccountPkp.publicKey,
+      pubKey: pubkey || ctx.aliceViemAccountPkp.pubkey,


:nit: Would be preferable to enforce that the caller always passes in the expected pubkey vs. having an implicit fallback

This makes sense - the pubkey || was added only for custom auth, but rearranging it to have the pkp-sign.ts NOT binding to a particular account makes sense!

MaximusHaximus · 2025-09-23T12:49:35Z

packages/e2e/src/helper/tests/encrypt-decrypt-flow.ts

+    expect(decryptedStringResponse).toBeDefined();
+    expect(decryptedStringResponse.convertedData).toBe(stringData);

    // Test 6: Decrypt JSON data (traditional method)


:nit: It would be preferable if these were actually different tests instead of just described as such by comments; e.g. 1 describe per type of encrypt/decrypt flow, with a case for encrypting and a case for decrypting

Agreed - so we can pinpoint which test failed exactly instead of the whole flow.

packages/artillery/project.json

MaximusHaximus · 2025-09-23T12:53:51Z

package.json

-    "@nx/plugin": "17.3.0",
-    "@nx/react": "17.3.0",
-    "@nx/web": "17.3.0",
+    "@nx/esbuild": "21.2.1",


💖 New hotness.

- raise all workspace TS configs (including wasm) to ES2022 to match emitted code - add missing noble/ethers dependencies and tighten e2e peer/dev requirements so pnpm resolves - re-export PKP managers/types from networks and cast chainApi mint functions for strong typing - switch PKP storage provider interface to PKPData and drop the unused ethers provider - reuse existing Alice/Eve contexts in e2e helpers/specs to satisfy the new typings

Copilot

Pull Request Overview

This PR migrates the build system from Bun to PNPM, upgrades NX from 17.3.0 to 21.2.1, and modernizes TypeScript configuration to use ES2022. The migration also includes restructuring test configurations to use Jest instead of Bun's test runner and updating package dependencies across the monorepo.

Migrates package manager from Bun to PNPM with workspace configuration
Upgrades NX to v21.2.1 and TypeScript target to ES2022
Restructures testing to use Jest with proper ESM configuration and babel transforms

Reviewed Changes

Copilot reviewed 83 out of 100 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tsconfig.base.json	Updates TypeScript target and module from ES2020 to ES2022
package.json	Removes Bun-specific scripts and adds Jest-based E2E testing configuration
jest.e2e.config.ts	Adds comprehensive Jest configuration for E2E testing with babel transforms
packages/e2e/src/*	Restructures E2E tests to use standard Jest expectations instead of custom assertions
packages/networks/src/*	Updates type definitions and removes references to deprecated PKPInfo interface
babel.config.json	Adds Babel configuration for Jest TypeScript/ESM support
pnpm-workspace.yaml	Adds PNPM workspace configuration

Comments suppressed due to low confidence (1)

packages/networks/src/networks/vNaga/shared/managers/contract-manager/createContractsManager.ts:1

The refactored function removes the comprehensive null/undefined check and error handling logic that was present in the original implementation. The original code had more detailed error messages and validation. Consider preserving the more robust error handling to maintain code reliability.

import type { Account, Chain, WalletClient, Client } from 'viem';

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

packages/networks/src/networks/vNaga/shared/managers/contract-manager/createContractsManager.ts

packages/auth-services/src/queue-manager/src/bullmqSetup.ts

packages/types/src/lib/interfaces.ts

packages/types/src/lib/types.ts

jest.e2e.config.ts

…s-93-refactor-bun-pnpm

Ansonhkg added 21 commits September 15, 2025 14:12

fix(package.json): upgraded esbuild to 0.19.2 to satisfy Nx, removed …

567402c

…typedoc-theme-hierarchy because it blocked upgrades, and moved typedoc to 0.28.12 for compatibility with TypeScript 5.8+.

refactor(package.json): bun to npm

85bf7ac

fix(e2e): add force exit for npm e2e test

f64f3e6

fix(e2e): add force exit for npm e2e test

77586c1

refactor(artillery): migrate from using Bun to npm

e2fca2a

fix(package): fix a peer-dependency conflicts where [email protected]…

30bf9da

… only supports `babel-jest@^.29`.

refactor(pnpm): migrate from package-lock.json to pnpm-lock.yaml and …

2e53cfc

…update reset script in package.json

chore(package): remove unused dependencies and scripts

9ed7fe1

chore(scripts): remove unused script

0dad0a8

fix(e2e): test should work again

28afe72

chore: remove unused tools configuration and scripts

bac5bb2

refactor(contract-manager): improve type handling for wallet clients

9d8d389

refactor(e2e): moved ./e2e inside ./packages/e2e

0edeb1f

refactor(e2e): moved ./e2e inside ./packages/e2e

8a8163f

chore: upload lock file

151df69

refactor(artillery): update command scripts to include logging option…

d602829

…s and remove module type; add TypeScript configuration

chore(package):

0a1d6fb

- remove unused artillery scripts - and update test command in package.json - remove unused packages

Ansonhkg had a problem deploying to Health Check September 15, 2025 14:12 — with GitHub Actions Failure

Ansonhkg changed the title ~~Feature/jss 93 refactor bun pnpm~~ [Refactor] [bun > pnpm] Feature/jss 93 refactor bun pnpm Sep 15, 2025

Ansonhkg self-assigned this Sep 15, 2025

Ansonhkg requested review from glitch003, FedericoAmura, spacesailor24, MaximusHaximus, DashKash54 and awisniew207 September 15, 2025 15:20

fix(networks, schemas): fix build by adding the required StrictAuth d…

f59121d

…ata. This changes are added in #891 and #892 but they are pull in so this PR can build

Ansonhkg had a problem deploying to Health Check September 15, 2025 15:24 — with GitHub Actions Failure

Ansonhkg added 3 commits September 15, 2025 16:32

refactor(e2e/artillery):

8195ba6

- moved inline import to top of the file - migrated to jest syntax - export utils from e2e and consumed by artillery - moved artillery as its own package - migrated script commands

refactor(package): remove artillery scripts as we have nx commands al…

25eb2a0

…ready

fix(e2e): update auth context types in e2e tests to use AuthContext f…

1c18085

…or better type safety

Ansonhkg changed the title ~~[Refactor] [bun > pnpm] Feature/jss 93 refactor bun pnpm~~ ⭐️ [Refactor] [bun > pnpm] Feature/jss 93 refactor bun pnpm Sep 15, 2025

glitch003 approved these changes Sep 15, 2025

View reviewed changes

fix(e2e): #881 (comment)

9abb08f

Ansonhkg mentioned this pull request Sep 18, 2025

🐥 [Canary] Naga #902

Draft

Ansonhkg added 3 commits September 18, 2025 23:59

chore(package): update pnpm-lock.yaml

dfe87bc

Merge branch 'feature/jss-93-refactor-bun-pnpm' of github.com:LIT-Pro…

f7b6ea5

…tocol/js-sdk into feature/jss-107-refactor-move-e2e-and-artillery-into-packages

Merge pull request #891 from LIT-Protocol/feature/jss-107-refactor-mo…

a388b28

…ve-e2e-and-artillery-into-packages Feature/jss 107 refactor move e2e and artillery into packages

Ansonhkg had a problem deploying to Health Check September 18, 2025 23:04 — with GitHub Actions Failure

MaximusHaximus approved these changes Sep 23, 2025

View reviewed changes

Ansonhkg added 2 commits September 25, 2025 00:48

fix: #889 (comment)

838bd2f

Copilot AI review requested due to automatic review settings September 25, 2025 00:20

Copilot AI reviewed Sep 25, 2025

View reviewed changes

Merge branch 'naga' of github.com:LIT-Protocol/js-sdk into feature/js…

cb396ca

…s-93-refactor-bun-pnpm

Ansonhkg had a problem deploying to Health Check September 25, 2025 00:37 — with GitHub Actions Failure

Ansonhkg added 2 commits September 25, 2025 01:37

chore: update @lit-protocol/contracts version to 0.5.1 in pnpm-lock.yaml

aa3206f

fmt

d9b654e

Ansonhkg had a problem deploying to Health Check September 25, 2025 00:38 — with GitHub Actions Failure

fix: remove redundant CustomAuthDataSchema definition

138f0c7

Ansonhkg had a problem deploying to Health Check September 25, 2025 00:54 — with GitHub Actions Failure

		import { getOrCreatePkp } from '@lit-protocol/e2e/src/helper/pkp-utils';
		import * as NetworkManager from '@lit-protocol/e2e/src/helper/NetworkManager';

⭐️ [Refactor] [bun > pnpm] Feature/jss 93 refactor bun pnpm #889

Are you sure you want to change the base?

⭐️ [Refactor] [bun > pnpm] Feature/jss 93 refactor bun pnpm #889

Uh oh!

Conversation

Ansonhkg commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHAT

Do not merge this until these are merged

UPDATE 22/09/2025

Uh oh!

glitch003 left a comment

Choose a reason for hiding this comment

Uh oh!

MaximusHaximus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ansonhkg commented Sep 15, 2025 •

edited

Loading