-
Notifications
You must be signed in to change notification settings - Fork 387
Add distribution detection pattern #6028
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
Merged
Merged
Conversation
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
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/12/2025, 04:12:27 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 10/12/2025, 04:27:10 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Merging now so others can use - please still leave any review if you have it and I'll address in followups. |
Merged
arjansingh
added a commit
that referenced
this pull request
Oct 14, 2025
## What's Changed ### 🚀 Features - Add MediaAssetCard presentation components (#5878) - Make Vue nodes' outputs/previews responsively sized and work with node resizing (#5970) - Allow connection to subgraphIOs in vue mode (#6016) - Add distribution detection pattern (#6028) - Make nodeData.widgets reactive (#6019) ### 🐛 Bug Fixes - Fix FLOAT widget incrementing broken & disabled state styles on widget number input (Vue) (#6036) - Fix Vue node border styles in different states (executing, error, selected) (#6018) - Fix Vue node opacity conditions (user node opacity, bypass state, muted state) (#6022) - Fix: emit layout change for batch node bounds (#5939) - Safer restoration of widgets_values on subgraph nodes (#6015) - Fix(execution): reset progress state after runs to unfreeze tab title/favicon (main) (#6026) - Use type check instead of cast (#6041) ### 🎨 Style & Design - [style] match widget border/outline styles with designs (#6021) - [style] make Vue widget/slot/label width and spacing align with designs (#6023) ### ♿ Accessibility - Add aria labels on vue node widgets (#6032) ### 🔧 Maintenance - [refactor] adjust Vue node fixtures to not be coupled to Litegraph (#6033) - [refactor] reorganize devtools test nodes into modules (#6020) ### 🧪 Testing - [test] add browser test for control+a selection of Vue nodes (#6031) ### 🔄 CI/CD - [ci] fix update locales workflow (#6017) **Full Changelog**: v1.29.1...v1.29.2 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6045-1-29-2-28c6d73d3650817a8c36fba944ce69a8) by [Unito](https://www.unito.io) --------- Co-authored-by: arjansingh <[email protected]> Co-authored-by: github-actions <[email protected]>
arjansingh
pushed a commit
that referenced
this pull request
Oct 16, 2025
## Summary Establishes distribution-specific code pattern using compile-time constants and dead code elimination. Demonstrates with Help Center by hiding extension manager and update buttons in cloud distribution. Below commentary makes assumption that minifcation and tree-shaking is enabled (which isn't true yet, but will be eventually). ## Changes - **What**: Added `src/platform/distribution/types.ts` with distribution detection via `__DISTRIBUTION__` variable - **Build**: Vite replaces `__DISTRIBUTION__` at build time using environment variables - **Tree-shaking**: All code not relevant to target distribution is DCR'd and eliminated from bundle - **Example**: Help Center hides "Manager Extension" menu item and "Update" buttons in cloud builds ## Pattern This PR defines a `__DISTRIBUTION__` variable which gets replaced at build time by Vite using environment variables. All code not relevant to the given distribution is then DCR'd and tree-shaken. For simple cases (like this Help Center PR), import `isCloud` and use compile-time conditionals: ```typescript import { isCloud } from '@/platform/distribution/types' if (!isCloud) { items.push({ key: 'manager', action: async () => { await useManagerState().openManager({ ... }) } }) } ``` The code is DCR'd at build time so there's zero runtime overhead - we don't even incur the `if (isCloud)` cost because Terser eliminates it. For complex services later, we'll add interfaces and use an index.ts that exports different implementations under the same alias per distribution. It will resemble a DI container but simpler since we don't need runtime discovery like backend devs do. This guarantees types and makes testing easier. Example for services: ```typescript // src/platform/storage/index.ts import { isCloud } from '@/platform/distribution/types' if (isCloud) { export { CloudStorage as StorageService } from './cloud' } else { export { LocalStorage as StorageService } from './local' } ``` Example for component variants: ```typescript // src/components/downloads/index.ts import { isCloud } from '@/platform/distribution/types' if (isCloud) { export { default as DownloadButton } from './DownloadButton.cloud.vue' } else { export { default as DownloadButton } from './DownloadButton.desktop.vue' } ``` ## Implementation Details Distribution types (`src/platform/distribution/types.ts`): ```typescript type Distribution = 'desktop' | 'localhost' | 'cloud' declare global { const __DISTRIBUTION__: Distribution } const DISTRIBUTION: Distribution = __DISTRIBUTION__ export const isCloud = DISTRIBUTION === 'cloud' ``` Vite configuration adds the define: ```typescript const DISTRIBUTION = (process.env.DISTRIBUTION || 'localhost') as | 'desktop' | 'localhost' | 'cloud' export default defineConfig({ define: { __DISTRIBUTION__: JSON.stringify(DISTRIBUTION) } }) ``` ## Build Commands ```bash pnpm build # localhost (default) DISTRIBUTION=cloud pnpm build # cloud DISTRIBUTION=desktop pnpm build # desktop ``` ## Future Applications This pattern can be used with auth or telemetry services - which will guarantee all the telemetry code, for example, is not even in the code distributed in OSS Comfy whatsoever while still being able to develop off `main`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6028-Add-distribution-detection-pattern-28a6d73d365081b08767d395472cd1bc) by [Unito](https://www.unito.io)
arjansingh
added a commit
that referenced
this pull request
Oct 17, 2025
## Summary Updates with cloud specific features merged into `main`. Notable changes include the `DISTRIBUTION=cloud` changes. Will also be changing cloud build workflow to build with that flag in Comfy-Org/cloud#1043 ## Changes - bb61d98 feat: AssetCard tweaks (#6085) - 05f7352 fix terminal style (#6056) - d5fa221 Add distribution detection pattern (#6028) - 6c36aaa feat: Improve MediaAssetCard video controls and add gallery view (#6065) - 6944ef0 fix Cloudbadge (#6063) - 6764f8d Badge for cloud environment (#6048) --------- Co-authored-by: Johnpaul Chiwetelu <[email protected]> Co-authored-by: Jin Yi <[email protected]> Co-authored-by: Claude <[email protected]> Co-authored-by: Christian Byrne <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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.
Summary
Establishes distribution-specific code pattern using compile-time constants and dead code elimination. Demonstrates with Help Center by hiding extension manager and update buttons in cloud distribution.
Below commentary makes assumption that minifcation and tree-shaking is enabled (which isn't true yet, but will be eventually).
Changes
src/platform/distribution/types.ts
with distribution detection via__DISTRIBUTION__
variable__DISTRIBUTION__
at build time using environment variablesPattern
This PR defines a
__DISTRIBUTION__
variable which gets replaced at build time by Vite using environment variables. All code not relevant to the given distribution is then DCR'd and tree-shaken.For simple cases (like this Help Center PR), import
isCloud
and use compile-time conditionals:The code is DCR'd at build time so there's zero runtime overhead - we don't even incur the
if (isCloud)
cost because Terser eliminates it.For complex services later, we'll add interfaces and use an index.ts that exports different implementations under the same alias per distribution. It will resemble a DI container but simpler since we don't need runtime discovery like backend devs do. This guarantees types and makes testing easier.
Example for services:
Example for component variants:
Implementation Details
Distribution types (
src/platform/distribution/types.ts
):Vite configuration adds the define:
Build Commands
Future Applications
This pattern can be used with auth or telemetry services - which will guarantee all the telemetry code, for example, is not even in the code distributed in OSS Comfy whatsoever while still being able to develop off
main
.┆Issue is synchronized with this Notion page by Unito