-
Notifications
You must be signed in to change notification settings - Fork 618
[SDK] Fix: Process undefined checks in vite #7844
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
🦋 Changeset detectedLatest commit: 0d7e5e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds typeof process !== "undefined" guards to IS_DEV and IS_TEST in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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)
packages/thirdweb/src/utils/process.ts (1)
1-7: Optional: compute NODE_ENV once and derive flags from itReduces repeated property access and keeps the logic DRY while preserving behavior.
-export const IS_DEV = - typeof process !== "undefined" && - process.env && - (process.env.NODE_ENV === "development" || process.env.NODE_ENV === "test"); - -export const IS_TEST = typeof process !== "undefined" && process.env && process.env.NODE_ENV === "test"; +const NODE_ENV = + typeof process !== "undefined" && process.env ? process.env.NODE_ENV : undefined; +export const IS_DEV = NODE_ENV === "development" || NODE_ENV === "test"; +export const IS_TEST = NODE_ENV === "test";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/thirdweb/src/utils/process.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
packages/thirdweb/src/utils/process.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
packages/thirdweb/src/utils/process.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/thirdweb/src/utils/process.ts (3)
2-3: Robust guard prevents ReferenceError in non-Node runtimes — good changeAdding
typeof process !== "undefined"before touchingprocess.envis correct and solves Vite/browser crashes.
6-6: IS_TEST now correctly guarded and consistent with IS_DEVThis mirrors the new pattern and avoids runtime errors where
processis absent.
1-7: Confirmed: All rawprocess.references are either in Node‐only contexts or properly guarded
- Node scripts (
scripts/hotlink/*.mjs), build/config files (vite.config.ts,vitest.config.ts) and test suites intentionally useprocess.*in a Node environment.- Runtime library code under
packages/thirdweb/srconly accessesprocessvia the guarded helpers in:
utils/process.ts(checkstypeof process !== "undefined"before usingprocess.env)utils/platform.ts(checkstypeof process !== "undefined"before usingprocess.versions)utils/fetch.ts(usesIS_DEV/IS_TESTfromprocess.ts, no rawprocess.calls)- React/Next.js applications reference
process.env.*during build time via webpack/Next.js DefinePlugin, so no unguarded runtimeprocessusage in browser bundles.No unguarded
process.calls remain in code that runs in the browser—no further changes needed.
26213c4 to
9999b08
Compare
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)
.changeset/fine-zoos-own.md (1)
5-5: Fix typo and clarify the message (mention Vite).Small polish to the changeset description for spelling and clarity.
Apply this diff:
-Fix SDK resolution in environements where process is undefined +Fix SDK resolution in environments where `process` is not defined (e.g., Vite).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fine-zoos-own.md(1 hunks)packages/thirdweb/src/utils/process.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/thirdweb/src/utils/process.ts
🧰 Additional context used
🪛 LanguageTool
.changeset/fine-zoos-own.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...dweb": patch --- Fix SDK resolution in environements where process is undefined
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Unit Tests
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
.changeset/fine-zoos-own.md (1)
1-3: Changeset front-matter looks valid.The package key and version bump type are correctly specified for Changesets.
size-limit report 📦
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7844 +/- ##
==========================================
- Coverage 56.34% 56.33% -0.01%
==========================================
Files 905 905
Lines 58830 58834 +4
Branches 4149 4151 +2
==========================================
- Hits 33147 33146 -1
- Misses 25577 25582 +5
Partials 106 106
🚀 New features to boost your workflow:
|
Merge activity
|
<!--
## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes"
If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000):
## Notes for the reviewer
Anything important to call out? Be sure to also clarify these in your comments.
## How to test
Unit tests, playground, etc.
-->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR addresses the resolution of the SDK in environments where `process` is undefined, ensuring that the checks for development and test environments are more robust.
### Detailed summary
- Updated the `IS_DEV` constant to check if `process` is defined before accessing its properties.
- Modified the `IS_TEST` constant similarly to ensure it only evaluates when `process` is defined.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Guarded environment checks to avoid runtime errors where Node globals are absent, improving stability across browsers, serverless, and edge runtimes.
* More reliable detection of development and test modes, reducing unexpected crashes during bundling, CI testing, and client-side use.
* **Chores**
* Added release metadata so the fix will be published as a patch.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
7a01b90 to
0d7e5e5
Compare
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)
.changeset/fine-zoos-own.md (1)
5-5: Fix typo and improve clarity in changeset description.Correct “environements” → “environments” and consider clarifying context (e.g., Vite) and wrapping process in backticks for readability.
Apply this diff:
-Fix SDK resolution in environements where process is undefined +Fix SDK resolution in environments where `process` is undefined (e.g., Vite)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fine-zoos-own.md(1 hunks)packages/thirdweb/src/utils/process.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/thirdweb/src/utils/process.ts
🧰 Additional context used
🪛 LanguageTool
.changeset/fine-zoos-own.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...dweb": patch --- Fix SDK resolution in environements where process is undefined
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
.changeset/fine-zoos-own.md (1)
1-3: Changeset frontmatter looks correct.Patch bump for "thirdweb" is appropriate for this fix.
PR-Codex overview
This PR focuses on fixing the resolution of the SDK in environments where
processis undefined, ensuring that the code correctly checks for the existence ofprocessand its properties.Detailed summary
IS_DEVto check ifprocessis defined before accessing its properties.IS_TESTsimilarly to ensureprocessis defined before checkingNODE_ENV.Summary by CodeRabbit
Bug Fixes
Chores