Enhance browser compatibility and update Vite configuration#486
Enhance browser compatibility and update Vite configuration#486kavindadimuthu wants to merge 2 commits intoasgardeo:mainfrom
Conversation
…Vite configuration for pre-bundling external dependencies
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR refactors the Buffer polyfill strategy by removing build-time banner/footer injection from esbuild configuration and moving the runtime global assignment to the browser package's index.ts entrypoint. Additionally, it introduces Nuxt module configuration for pre-bundling CJS-dependent packages in the Vite build. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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. Review rate limit: 0/1 reviews remaining, refill in 46 minutes and 3 seconds.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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxt/src/module.ts`:
- Around line 309-316: The current optimizeDeps include list in the deps array
references transitive packages (`@asgardeo/javascript`, base64url, cross-fetch,
fast-sha256) that are not declared in packages/nuxt/package.json; update the
module by either (A) adding these packages as explicit direct dependencies in
package.json so strict-mode package managers resolve them correctly, or (B)
remove them from the deps array and only include the top-level packages
('@asgardeo/browser' / '@asgardeo/node') if you confirm they bundle those
transitive modules at runtime; locate the deps constant in module.ts and ensure
package.json and the deps array are consistent to avoid strict-install failures.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 705c05e4-f359-49ff-845c-25a0e2e1dfe4
📒 Files selected for processing (3)
packages/browser/esbuild.config.mjspackages/browser/src/index.tspackages/nuxt/src/module.ts
💤 Files with no reviewable changes (1)
- packages/browser/esbuild.config.mjs
| const deps = [ | ||
| '@asgardeo/browser', | ||
| '@asgardeo/javascript', | ||
| '@asgardeo/vue', | ||
| 'base64url', | ||
| 'cross-fetch', | ||
| 'fast-sha256', | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
from pathlib import Path
nuxt_pkg = Path("packages/nuxt/package.json")
required = [
"@asgardeo/browser",
"@asgardeo/javascript",
"@asgardeo/vue",
"base64url",
"cross-fetch",
"fast-sha256",
]
data = json.loads(nuxt_pkg.read_text())
deps = data.get("dependencies", {})
peer = data.get("peerDependencies", {})
print(f"Checking {nuxt_pkg}:")
for name in required:
declared = name in deps or name in peer
where = "dependencies" if name in deps else ("peerDependencies" if name in peer else "NOT DECLARED")
print(f" - {name}: {where}")
print("\nIf any entry is NOT DECLARED, validate whether forcing it in optimizeDeps.include is safe for strict package managers.")
PYRepository: asgardeo/javascript
Length of output: 415
🏁 Script executed:
# Get context around lines 309-316 in the module.ts file
head -n 330 packages/nuxt/src/module.ts | tail -n 40Repository: asgardeo/javascript
Length of output: 1385
🏁 Script executed:
# Search for imports or usage of the undeclared packages in the nuxt module
rg -l "@asgardeo/javascript|base64url|cross-fetch|fast-sha256" packages/nuxt/srcRepository: asgardeo/javascript
Length of output: 91
🏁 Script executed:
# Check if these undeclared packages might be transitive dependencies
rg '"@asgardeo/javascript"|"base64url"|"cross-fetch"|"fast-sha256"' packages/nuxt/package.json packages/nuxt/package-lock.json 2>/dev/null || echo "Checking if packages are transitive..."Repository: asgardeo/javascript
Length of output: 102
🏁 Script executed:
# Check if the undeclared packages are listed as external dependencies of `@asgardeo/browser`
# Look at what `@asgardeo/browser` depends on
cat packages/browser/package.json | jq '.dependencies, .peerDependencies, .optionalDependencies' 2>/dev/null || echo "Checking browser package..."Repository: asgardeo/javascript
Length of output: 366
🏁 Script executed:
# Look for `@asgardeo/javascript` as a dependency in browser or vue packages
rg '"@asgardeo/javascript"' packages/browser/package.json packages/vue/package.json 2>/dev/null || echo "Not found in direct search"Repository: asgardeo/javascript
Length of output: 136
🏁 Script executed:
# Check if these transitive packages are actually listed in package-lock.json or the monorepo structure
fd package.json | xargs grep -l "base64url\|cross-fetch\|fast-sha256" 2>/dev/null | head -10Repository: asgardeo/javascript
Length of output: 120
🏁 Script executed:
# Check `@asgardeo/javascript`'s dependencies to see if it declares base64url and fast-sha256
cat packages/javascript/package.json | jq '.dependencies, .peerDependencies' 2>/dev/null || echo "Checking javascript package..."Repository: asgardeo/javascript
Length of output: 146
🏁 Script executed:
# Check `@asgardeo/vue`'s dependencies
cat packages/vue/package.json | jq '.dependencies, .peerDependencies' 2>/dev/null || echo "Checking vue package..."Repository: asgardeo/javascript
Length of output: 236
🏁 Script executed:
# Search for cross-fetch in all package.json files
grep -r "cross-fetch" packages/ --include="package.json"Repository: asgardeo/javascript
Length of output: 118
🏁 Script executed:
# Check all dependencies of `@asgardeo/browser` more carefully including dev deps
cat packages/browser/package.json | jq '.' 2>/dev/null | head -80Repository: asgardeo/javascript
Length of output: 2116
🏁 Script executed:
# Search for where cross-fetch is actually used in the nuxt module or imports
rg "cross-fetch" packages/nuxt/Repository: asgardeo/javascript
Length of output: 116
🏁 Script executed:
# Check if cross-fetch should be coming from `@asgardeo/browser`'s dependencies
rg "cross-fetch" packages/browser/Repository: asgardeo/javascript
Length of output: 45
🏁 Script executed:
# Let's check the full nuxt package.json to see all declared dependencies
cat packages/nuxt/package.json | jq '.dependencies, .peerDependencies' 2>/dev/nullRepository: asgardeo/javascript
Length of output: 283
🏁 Script executed:
# Double-check where cross-fetch is actually imported/used in the codebase
rg "cross-fetch|from.*cross-fetch|import.*cross-fetch" packages/Repository: asgardeo/javascript
Length of output: 261
🏁 Script executed:
# Check if cross-fetch is used in `@asgardeo/browser` source code
rg "cross-fetch" packages/browser/src/Repository: asgardeo/javascript
Length of output: 45
🏁 Script executed:
# Final verification: confirm `@asgardeo/node` is in nuxt dependencies and check its full package.json
cat packages/nuxt/package.json | jq '.dependencies | keys'Repository: asgardeo/javascript
Length of output: 163
🏁 Script executed:
# Verify the dependency chain: nuxt -> `@asgardeo/node` -> cross-fetch
echo "=== `@asgardeo/node` dependencies ===" && cat packages/node/package.json | jq '.dependencies | keys'Repository: asgardeo/javascript
Length of output: 251
🏁 Script executed:
# Verify the dependency chain: nuxt -> `@asgardeo/browser` -> base64url, fast-sha256, `@asgardeo/javascript`
echo "=== `@asgardeo/browser` dependencies ===" && cat packages/browser/package.json | jq '.dependencies | keys'Repository: asgardeo/javascript
Length of output: 285
These packages are transitive dependencies; verify strict mode compatibility.
The packages @asgardeo/javascript, base64url, cross-fetch, and fast-sha256 are not declared in packages/nuxt/package.json but are transitive dependencies of the declared @asgardeo/browser and @asgardeo/node packages. While this ensures they are resolvable through the dependency tree, including them in optimizeDeps.include without explicit declaration may cause issues in strict package manager configurations. Verify this approach works correctly with strict mode installs, or alternatively declare these as direct dependencies if the Nuxt module explicitly depends on them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/module.ts` around lines 309 - 316, The current optimizeDeps
include list in the deps array references transitive packages
(`@asgardeo/javascript`, base64url, cross-fetch, fast-sha256) that are not
declared in packages/nuxt/package.json; update the module by either (A) adding
these packages as explicit direct dependencies in package.json so strict-mode
package managers resolve them correctly, or (B) remove them from the deps array
and only include the top-level packages ('@asgardeo/browser' / '@asgardeo/node')
if you confirm they bundle those transitive modules at runtime; locate the deps
constant in module.ts and ensure package.json and the deps array are consistent
to avoid strict-install failures.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. |
2 participants
Purpose
This pull request improves compatibility and reliability for browser and Nuxt environments by adjusting how Node.js polyfills are handled and ensuring proper dependency pre-bundling with Vite. The most important changes are grouped below:
Browser polyfill handling:
Bufferon thewindowobject from the esbuild configuration (esbuild.config.mjs— previously inbannerandfooter) directly into the browser entry point (src/index.ts). This ensuresBufferis always available globally in browser builds, preventing issues with libraries that expect Node polyfills. [1] [2] [3]Nuxt/Vite dependency handling:
@asgardeo/browser,@asgardeo/javascript,@asgardeo/vue,base64url,cross-fetch,fast-sha256) for client builds. This prevents "Export 'X' is not defined in module" errors caused by Vite serving these packages as raw CJS from disk. [1] [2]Related Issues
Related PRs
Checklist
Security checks
Summary by CodeRabbit
Refactor
Chores