-
-
Notifications
You must be signed in to change notification settings - Fork 600
fix: Missing dependencies in transpiled browser code #2812
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
Conversation
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe pull request adds a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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: 0
🧹 Nitpick comments (1)
package.json (1)
35-36: Recommend: Add brief explanatory comment in PR description.The PR's "Approach" section is empty, and the rationale for why these dependencies are now required is unclear. This makes it difficult for reviewers to assess the change. Please update the PR description to explain:
- Why
crypto-jsis needed- Why
idb-keyvalmoved from dev to runtime dependencies- Whether this is fixing a missing import or adding new functionality
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.gitignore(1 hunks)package.json(1 hunks)
🔇 Additional comments (5)
.gitignore (1)
17-18: Approve .gitignore addition.Adding the
.claudedirectory to.gitignoreis appropriate for isolating AI agent artifacts from version control.package.json (4)
35-35: Verifycrypto-jsis actually used in the codebase.Line 35 adds
crypto-js@4.2.0as a runtime dependency, but there is no evidence in the PR description or approach section explaining where it is imported or used. Adding unused dependencies increases bundle size and maintenance burden.Search the codebase for imports and usages of
crypto-js(e.g.,import ... from 'crypto-js',require('crypto-js'), orCryptoJSreferences). If the dependency is not actually used in production code, remove it from package.json.
35-36: Critical: PR issue linkage is missing—clarify which issue this fixes.The PR description shows "Closes: FILL_THIS_OUT", which means the associated issue is not linked. This makes it impossible to verify whether adding these dependencies is the correct fix. Please fill in the issue number or provide clear context for why
crypto-js@4.2.0andidb-keyval@6.2.2are required at runtime.Additionally, clarify why
idb-keyvalwas moved from devDependencies to runtime dependencies and confirm both packages are actually imported in the codebase.
36-36: Verifyidb-keyvalis used in production code to justify moving from devDependencies.Line 36 adds
idb-keyval@6.2.2as a runtime dependency. Whileidb-keyvalis designed as a runtime library for IndexedDB access in the browser and should be independencieswhen used in production, you need to confirm this package is actually imported and used in production source code (not just in tests or build tooling). If this is a new addition to production code, clarify in the PR description why this IndexedDB integration is necessary.
35-36: crypto-js@4.2.0 is the patched version and secure; no vulnerabilities found for idb-keyval@6.2.2.crypto-js@4.2.0 actually contains the fix for CVE-2023-46233 (PBKDF2 weakness patched October 2023), not a vulnerable version. Both packages are currently free of known security advisories and compatible with Node 18–24. Note: crypto-js has low maintenance activity; consider using Node's native
cryptomodule as an alternative where feasible.Likely an incorrect or invalid review comment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #2812 +/- ##
=======================================
Coverage 99.88% 99.88%
=======================================
Files 64 64
Lines 6222 6222
Branches 1473 1473
=======================================
Hits 6215 6215
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
## [7.1.2-alpha.1](7.1.1...7.1.2-alpha.1) (2025-12-01) ### Bug Fixes * Missing dependencies in transpiled browser code ([#2812](#2812)) ([c7359bb](c7359bb))
|
🎉 This change has been released in version 7.1.2-alpha.1 |
## [7.1.2](7.1.1...7.1.2) (2025-12-01) ### Bug Fixes * Missing dependencies in transpiled browser code ([#2812](#2812)) ([c7359bb](c7359bb))
|
🎉 This change has been released in version 7.1.2 |
Pull Request
Issue
In #2803, two packages were incorrectly identified as unused:
However, both packages are still being imported in the transpiled browser code:
When Parse Dashboard installs the Parse SDK as a dependency, these packages are missing, causing webpack to fail with "Module not found" errors.
Approach
Add both packages as regular dependencies:
For
idb-keyvaloptionalDependenciesis not the right solution because:The correct fix is to make
crypto-jsa regular dependency because:This is evidenced by the fact that the build process creates separate bundles for each environment, and each environment only bundles what it actually imports.