Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![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)
package.json (1)
291-352:⚠️ Potential issue | 🟡 MinorpeerDependency bump from
^8.3.2→^14.0.0is a breaking change for consumers.Any downstream app pinning
uuidat v8–v13 will now see a peer-dep mismatch after consuming this release. That is an intentional trade-off here, but please make sure the release that ships this carries aBREAKING CHANGE:footer sosemantic-releaseemits a major version bump and the breaking nature is called out in the changelog.Also worth noting:
uuidis used internally only insrc/elements/common/logger/Logger.js(for a one-timeSESSION_ID), not exposed on the public API surface. It's debatable whether it needs to be inpeerDependenciesat all versus a plaindependency. Not blocking for this PR, but worth revisiting separately.Based on learnings: the team is comfortable raising peerDependency minimums when upgrading packages, even if it's a breaking change for consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 291 - 352, The package bump of peerDependency "uuid" from ^8.3.2 to ^14.0.0 is a breaking change that will cause peer-dep mismatches for downstream apps; update the release notes to include a "BREAKING CHANGE:" footer so semantic-release triggers a major version and documents the change, and consider moving "uuid" from peerDependencies to dependencies if it's only used internally (see src/elements/common/logger/Logger.js and the SESSION_ID usage) to avoid imposing the upgrade on consumers—implement at least the BREAKING CHANGE footer in the release that contains this package.json change, and optionally change the package.json entry for "uuid" from peerDependencies to dependencies if you decide to keep it internal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 354-361: The "resolutions" entry forcing "uuid": "^14.0.0" is
unsafe because uuid@14 is ESM-only and will break CommonJS transitive consumers;
update the package.json "resolutions" block to pin uuid to a CommonJS-compatible
release (e.g., replace the uuid line with "uuid": "^8.3.2" or another
v8/v3-compatible semver), remove the ^14.0.0 entry if you need ESM-only usage,
then run the end-to-end checks (yarn start / yarn build:dev:dist and yarn cy:run
or yarn test:e2e) and confirm CI Node version matches engines.node (>=20) before
merging.
---
Outside diff comments:
In `@package.json`:
- Around line 291-352: The package bump of peerDependency "uuid" from ^8.3.2 to
^14.0.0 is a breaking change that will cause peer-dep mismatches for downstream
apps; update the release notes to include a "BREAKING CHANGE:" footer so
semantic-release triggers a major version and documents the change, and consider
moving "uuid" from peerDependencies to dependencies if it's only used internally
(see src/elements/common/logger/Logger.js and the SESSION_ID usage) to avoid
imposing the upgrade on consumers—implement at least the BREAKING CHANGE footer
in the release that contains this package.json change, and optionally change the
package.json entry for "uuid" from peerDependencies to dependencies if you
decide to keep it internal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6748c62-141c-4982-afee-2b82b68c9a24
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json
| "resolutions": { | ||
| "draft-js/immutable": "^3.8.3", | ||
| "eslint-plugin-formatjs/**/minimatch": "^9.0.9", | ||
| "qs": "^6.14.1", | ||
| "serialize-javascript": "^7.0.5", | ||
| "tar": "^7.5.11" | ||
| "tar": "^7.5.11", | ||
| "uuid": "^14.0.0" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any other direct/indirect consumers of `uuid` in the tree and confirm only v14 resolves.
fd -HI -t f '^yarn\.lock$' --max-depth 2
rg -nP '^\s*uuid@' yarn.lock | head -50
# Confirm only one uuid version will be resolved after the bump
rg -nP '^uuid@[^:]+:\s*$' -A 2 yarn.lock | head -40
# Find first-party call sites for uuid to confirm none rely on removed APIs (e.g. default export, uuid/v4 deep import)
rg -nP --type=js --type=ts --type=tsx -C2 "from ['\"]uuid(/|['\"])|require\(['\"]uuid(/|['\"])"Repository: box/box-ui-elements
Length of output: 350
🌐 Web query:
Does uuid v14 still ship a CommonJS build usable via require('uuid').v4(), and what is the minimum supported Node.js version?
💡 Result:
No, uuid v14 does not ship a CommonJS build usable via require('uuid').v4. Starting with uuid@12, CommonJS support was removed, making it ESM-only. Users must use ESM syntax: import { v4 } from 'uuid';. The minimum supported Node.js version for v14 is 20.
Citations:
- 1: https://github.com/uuidjs/uuid/releases/tag/v14.0.0
- 2: https://github.com/uuidjs/node-uuid
- 3: https://www.npmjs.com/package/uuid
- 4: https://github.com/uuidjs/uuid
- 5: Drop CommonJS Support uuidjs/uuid#881
- 6: uuidjs/uuid@v13.0.0...v14.0.0
🏁 Script executed:
#!/bin/bash
# Search for uuid imports/requires more carefully without tsx type
rg -n "uuid" --type js --type ts -C 2 | grep -E "(require|import).*uuid|uuid.*from|uuid.*require" | head -30
# Also try a broader search
rg "uuid" --type js -lRepository: box/box-ui-elements
Length of output: 743
🏁 Script executed:
#!/bin/bash
# Check the actual usage patterns in the codebase
rg -n "from ['\"]uuid|require\(['\"]uuid" --type js --type ts -C 2Repository: box/box-ui-elements
Length of output: 484
🏁 Script executed:
#!/bin/bash
cat -n scripts/jest/jest.config.js | head -30Repository: box/box-ui-elements
Length of output: 1808
🏁 Script executed:
#!/bin/bash
# Search for require('uuid') patterns in source code (not node_modules)
rg "require\(['\"]uuid" src/ scripts/ --type js --type ts -nRepository: box/box-ui-elements
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about uuid usage
rg "uuid" -A 2 -B 2 --type js --type ts src/ | grep -E "(v3|v4|v5|CJS|ESM|CommonJS|ES Module)" | head -20Repository: box/box-ui-elements
Length of output: 202
Forcing uuid@14.0.0 will likely break transitive consumers; do not merge without testing.
uuid@14 is ESM-only (CommonJS support was removed starting with uuid@12). The transitive dependencies — cypress → @cypress/request → uuid and webpack-dev-server → sockjs → uuid — were authored against uuid@3.x/uuid@8.x and almost certainly use require('uuid'). Requiring an ESM-only module with CommonJS will fail at runtime.
The repo's direct usage is correct (import { v4 as uuidv4 } from 'uuid' in Logger.js), but the transitive breakage is real. Test end-to-end before merging:
yarn start/yarn build:dev:dist(webpack-dev-server + sockjs path)yarn cy:run(oryarn test:e2e) (cypress +@cypress/requestpath)
uuid@14 requires Node ≥ 20; the repo's engines.node: ">=20.x" is compatible, but verify CI runners are not still on Node 18.x.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 354 - 361, The "resolutions" entry forcing "uuid":
"^14.0.0" is unsafe because uuid@14 is ESM-only and will break CommonJS
transitive consumers; update the package.json "resolutions" block to pin uuid to
a CommonJS-compatible release (e.g., replace the uuid line with "uuid": "^8.3.2"
or another v8/v3-compatible semver), remove the ^14.0.0 entry if you need
ESM-only usage, then run the end-to-end checks (yarn start / yarn build:dev:dist
and yarn cy:run or yarn test:e2e) and confirm CI Node version matches
engines.node (>=20) before merging.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/jest/jest.config.js (1)
14-14:⚠️ Potential issue | 🟠 MajorRemove the
^uuid$moduleNameMapper entry — it conflicts with uuid v14's exports field.The
transformIgnorePatternswhitelist on line 31 already handles uuid correctly. The moduleNameMapper on line 14 is redundant and can interfere with Jest's export field resolution, potentially short-circuiting to a wrong entry point. Modern versions of uuid (v14+) and Jest (27+) handle exports natively; the mapping should be removed entirely.Current jest.config.js excerpt (lines 8–17)
moduleNameMapper: { 'box-ui-elements-locale-data': '<rootDir>/i18n/bundles/en-US.js', 'box-locale-data': '<rootDir>/node_modules/@box/cldr-data/locale-data/en-US', '\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': '<rootDir>/scripts/jest/mocks/fileMock.js', '\\.(css|less|scss|md)$': '<rootDir>/scripts/jest/mocks/styleMock.js', '^uuid$': require.resolve('uuid'), '^react-markdown$': '<rootDir>/scripts/jest/mocks/reactMarkdownMock.js', '^remark-gfm$': '<rootDir>/scripts/jest/mocks/remarkGfmMock.js', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/jest/jest.config.js` at line 14, Remove the '^uuid$' entry from moduleNameMapper in the jest configuration to avoid overriding uuid v14's export resolution; locate the moduleNameMapper object (the mapping that currently contains '^uuid$': require.resolve('uuid')) and delete that key/value pair so Jest uses uuid's native exports (transformIgnorePatterns already handles uuid).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/jest/jest.config.js`:
- Line 14: Remove the '^uuid$' entry from moduleNameMapper in the jest
configuration to avoid overriding uuid v14's export resolution; locate the
moduleNameMapper object (the mapping that currently contains '^uuid$':
require.resolve('uuid')) and delete that key/value pair so Jest uses uuid's
native exports (transformIgnorePatterns already handles uuid).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a0db23c-92eb-40a4-aad7-20149e0089fd
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.jsonscripts/jest/jest.config.js
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
1 participant
Upgrading UUID, must be >14.0.0
No significant breaking changes between 8 and 14 - most deal with dropping node support.
UUID v8 is introduced via direct dependency AND indirect
The resolutions is required as cypress/request is used within webpack-dev-server, and no version of cypress/request replaces uuid.
cypress-io/cypress#29775 (comment)
Summary by CodeRabbit
Chores
Tests