fix(sdk): add isBrowserEnvValue for tree-shaking#4454
Open
ScriptedAlchemy wants to merge 9 commits intomainfrom
Open
fix(sdk): add isBrowserEnvValue for tree-shaking#4454ScriptedAlchemy wants to merge 9 commits intomainfrom
ScriptedAlchemy wants to merge 9 commits intomainfrom
Conversation
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
🦋 Changeset detectedLatest commit: 2dc915f The changes in this PR will be included in the next version bump. This PR includes changesets to release 43 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 |
Open
5 tasks
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@module-federation/devtools
@module-federation/cli
create-module-federation
@module-federation/data-prefetch
@module-federation/dts-plugin
@module-federation/enhanced
@module-federation/error-codes
@module-federation/managers
@module-federation/manifest
@module-federation/metro
@module-federation/metro-plugin-rnc-cli
@module-federation/metro-plugin-rnef
@module-federation/modern-js-v3
@module-federation/retry-plugin
@module-federation/rsbuild-plugin
@module-federation/rspack
@module-federation/rspress-plugin
@module-federation/runtime
@module-federation/runtime-core
@module-federation/runtime-tools
@module-federation/sdk
@module-federation/third-party-dts-extractor
@module-federation/treeshake-frontend
@module-federation/treeshake-server
@module-federation/webpack-bundler-runtime
@module-federation/bridge-react
@module-federation/bridge-react-webpack-plugin
@module-federation/bridge-shared
@module-federation/bridge-vue3
@module-federation/inject-external-runtime-core-plugin
commit: |
Contributor
Bundle Size Report15 package(s) changed, 26 unchanged. Package dist + ESM entry
Bundle targets
Total dist (raw): 6.89 MB (+738 B (+0.0%)) Bundle sizes are generated with rslib (Rspack). Web/node bundles set ENV_TARGET and enable tree-shaking. Bare imports are externalized to keep sizes consistent with prior reporting, and assets are emitted as resources. |
ScriptedAlchemy
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82b5e2e668
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+11
to
+14
| const isBrowserEnvValue = | ||
| typeof ENV_TARGET !== 'undefined' | ||
| ? ENV_TARGET === 'web' | ||
| : typeof window !== 'undefined' && typeof window.document !== 'undefined'; |
There was a problem hiding this comment.
This change memoizes browser detection at module evaluation (isBrowserEnvValue) and then has isBrowserEnv() return that cached value, so imports that happen before DOM globals are attached (for example in SSR/test harness bootstraps that set window later) will permanently report non-browser and route runtime code down node-only paths. Previously isBrowserEnv() re-checked window/document each call, so this is a behavior regression in dynamic initialization contexts.
Useful? React with 👍 / 👎.
joshuaellis
approved these changes
Feb 17, 2026
joshuaellis
left a comment
joshuaellis
left a comment
There was a problem hiding this comment.
Yep, this has improved things significantly, the mf-vite package still needs some other changes, but this is a very good first step as any node branches collapse to no-ops when we apply the ENV_TARGET. Thanks!
|
Be great to get this merged and published so we can update the vite plugin upstream with the new release 🙏 |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refactors
isBrowserEnvfrom a function to a top-levelconstthat respectsENV_TARGETand includes a runtime fallback. This change enables bundlers to effectively tree-shake browser-specific code when targeting non-browser environments. All call sites acrossruntime-core,sdk, andbridgepackages have been updated to use the booleanisBrowserEnvdirectly.Related Issue
No specific issue linked, but addresses the general goal of improving tree-shaking for
isBrowserEnvusage.Types of changes
Checklist
fixes: #4452