fix(@deepnote/convert): replace uuid with crypto.randomUUID for CJS compatibility#214
fix(@deepnote/convert): replace uuid with crypto.randomUUID for CJS compatibility#214
Conversation
📝 WalkthroughWalkthroughThe convert package migrates from the external uuid library to Node.js's built-in crypto.randomUUID() for ID generation across all notebook conversion modules. The uuid dependency was removed from package.json, and all ID generation calls (in jupyter, marimo, percent, and quarto converters) now use crypto.randomUUID instead of uuid.v4. Test mocks were updated to reflect this change. Version bumped to 2.1.3. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
=======================================
Coverage 91.57% 91.57%
=======================================
Files 41 41
Lines 2041 2041
Branches 652 630 -22
=======================================
Hits 1869 1869
Misses 172 172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/convert/src/jupyter-to-deepnote.test.ts (1)
1372-1372: Stale test description.Test name references "uuid v4" but implementation now uses
crypto.randomUUID.Suggested fix
- it('uses default uuid v4 when no idGenerator provided', () => { + it('uses default crypto.randomUUID when no idGenerator provided', () => {
🤖 Fix all issues with AI agents
In `@packages/convert/src/percent-to-deepnote.test.ts`:
- Around line 13-34: Extract a dedicated type alias (e.g., MockedRandomUUID) for
the mocked randomUUID type instead of repeating the complex cast; update the
mock creation (mockRandomUUID) to be cast as that alias and assign
__resetCounter on it, and change getMockedRandomUUID() to return that alias (use
vi.mocked(crypto.randomUUID) as MockedRandomUUID). This simplifies typings while
keeping the existing mock behavior and __resetCounter helper.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/convert/package.jsonpackages/convert/src/jupyter-to-deepnote.test.tspackages/convert/src/jupyter-to-deepnote.tspackages/convert/src/marimo-to-deepnote.test.tspackages/convert/src/marimo-to-deepnote.tspackages/convert/src/percent-to-deepnote.test.tspackages/convert/src/percent-to-deepnote.tspackages/convert/src/quarto-to-deepnote.test.tspackages/convert/src/quarto-to-deepnote.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Place test files next to source files with.test.tsor.test.tsxextension
Use strict type checking in TypeScript files
Prefer type safety over convenience in TypeScript
Useconstfor immutable values
Avoidanytypes - use proper type definitions
Follow Biome's rules (configured inbiome.json)
Use literal keys instead of bracket notation when possible
Prefer single quotes for strings (except when avoiding escapes)
Keep code clean and readable following Biome standards
Files:
packages/convert/src/marimo-to-deepnote.test.tspackages/convert/src/quarto-to-deepnote.test.tspackages/convert/src/jupyter-to-deepnote.test.tspackages/convert/src/quarto-to-deepnote.tspackages/convert/src/marimo-to-deepnote.tspackages/convert/src/percent-to-deepnote.tspackages/convert/src/percent-to-deepnote.test.tspackages/convert/src/jupyter-to-deepnote.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest as the testing framework
Follow existing test patterns in the codebase (seepackages/blocks/src/blocks/*.test.ts)
Test edge cases, error handling, and special characters
Files:
packages/convert/src/marimo-to-deepnote.test.tspackages/convert/src/quarto-to-deepnote.test.tspackages/convert/src/jupyter-to-deepnote.test.tspackages/convert/src/percent-to-deepnote.test.ts
🔇 Additional comments (19)
packages/convert/package.json (1)
3-3: LGTM!Version bump and
uuidremoval are consistent with the migration tocrypto.randomUUID. Node ≥22.14.0 requirement more than covers the API availability.Also applies to: 37-43, 49-51
packages/convert/src/percent-to-deepnote.test.ts (2)
274-279: Correct mock reset.Clearing and resetting counter ensures test isolation.
374-381: LGTM.Same pattern applied consistently.
packages/convert/src/jupyter-to-deepnote.ts (3)
1-1: Clean import.Named import
randomUUIDis cleaner than the oldv4 as uuidv4pattern.
43-48: LGTM.Default to
randomUUIDis straightforward.
85-88: Consistent replacement.All ID generation points now use
randomUUID.Also applies to: 97-97, 122-122
packages/convert/src/quarto-to-deepnote.ts (2)
1-1: Consistent with other converters.Import and JSDoc update match the pattern.
Also applies to: 14-17
318-322: LGTM.All ID generation uses
randomUUIDnow.Also applies to: 352-365, 385-385
packages/convert/src/marimo-to-deepnote.ts (3)
1-1: Same pattern applied.Matches other converter files.
83-86: LGTM.JSDoc and default ID generator updated correctly.
Also applies to: 297-299
302-307: Consistent implementation.
idGeneratoroption propagates correctly through the conversion chain.Also applies to: 317-321
packages/convert/src/marimo-to-deepnote.test.ts (2)
1-30: Clean migration to crypto.randomUUID mock.The mock setup correctly preserves the actual crypto module while overriding only
randomUUID. The__resetCounterhook via type assertion is a practical approach for deterministic test IDs.
701-705: Reset pattern is consistent.Both
beforeEachblocks properly clear and reset the mock counter before each test.Also applies to: 837-840
packages/convert/src/jupyter-to-deepnote.test.ts (1)
1-32: Mock setup matches other test files.Consistent pattern for crypto.randomUUID mocking with reset capability.
packages/convert/src/quarto-to-deepnote.test.ts (2)
1-34: Consistent mock pattern.Same approach as other test files. Clean implementation.
450-453: Reset blocks are correctly placed.Also applies to: 534-536
packages/convert/src/percent-to-deepnote.ts (3)
1-1: Correct import.Using
node:cryptoprefix is idiomatic for Node.js built-ins.
14-16: JSDoc updated.Documentation reflects the new default.
159-159: All ID generation sites updated.Consistent use of
randomUUIDthroughout. Node ≥22.14.0 requirement ensures availability.Also applies to: 176-176, 183-183, 201-201
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Summary
uuidpackage with Node.js built-incrypto.randomUUID()to fix ESM/CJS compatibility issuesuuiddependency from package.jsonnode:cryptoinstead ofuuidProblem
When consuming
@deepnote/convertin CommonJS environments (e.g., Jest), the following error occurred:This happened because
uuid@13+is ESM-only, but the CJS build externalized it asrequire("uuid").Solution
Replaced
uuid.v4()withcrypto.randomUUID(), which is a Node.js built-in available since v14.17.0. Since the package requires Node ≥22.14.0, this is safe and removes the need for any external UUID dependency.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.