Skip to content

Conversation

@LittleSound
Copy link
Collaborator

@LittleSound LittleSound commented Oct 15, 2025

Summary

I believe tree-shaking does not affect the shim, and it should safe to enable it.

maybe we need to find the extension that will broke in this case.

Rendered Gzip Brotli
Before 16.69 MB 4.23 MB 3.54 MB
After 12.07 MB 3 MB 2.53 MB

@github-actions
Copy link

github-actions bot commented Oct 15, 2025

🎨 Storybook Build Status

Build completed successfully!

⏰ Completed at: 10/15/2025, 09:03:33 AM UTC

🔗 Links


🎉 Your Storybook is ready for review!

@github-actions
Copy link

github-actions bot commented Oct 15, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 10/15/2025, 09:18:26 AM UTC

📈 Summary

  • Total Tests: 498
  • Passed: 465 ✅
  • Failed: 0
  • Flaky: 3 ⚠️
  • Skipped: 30 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 456 / ❌ 0 / ⚠️ 3 / ⏭️ 30
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 6 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

@LittleSound LittleSound changed the title wip: test the tree shaking and minify wip: tree shaking and minify Oct 15, 2025
@LittleSound LittleSound changed the title wip: tree shaking and minify perf: tree shaking and minify Oct 15, 2025
@LittleSound LittleSound marked this pull request as ready for review October 15, 2025 08:50
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 15, 2025
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 15, 2025
@socket-security
Copy link

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedloglevel@​1.9.210010010078100
Addedtailwind-merge@​2.6.01001008695100
Addeduuid@​11.1.010010010088100
Addedjiti@​2.4.29910010089100
Addedsemver@​7.7.210010010090100
Addedzod@​3.24.110010010095100

View full report

@socket-security
Copy link

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
[email protected] has Obfuscated code.

Confidence: 0.94

Location: Package overview

From: pnpm-lock.yamlnpm/[email protected]

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected].

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@christian-byrne christian-byrne merged commit 6443070 into main Oct 17, 2025
30 checks passed
@christian-byrne christian-byrne deleted the rizumu/perf/cloud-minify branch October 17, 2025 01:44
christian-byrne pushed a commit that referenced this pull request Oct 17, 2025
## Summary

I believe tree-shaking does not affect [the
shim](https://github.com/Comfy-Org/ComfyUI_frontend/blob/rizumu/perf/cloud-minify/build/plugins/comfyAPIPlugin.ts#L73),
and it should safe to enable it.

maybe we need to find the extension that will broke in this case.


|   | Rendered | Gzip     | Brotli  |
|--------|-----------|----------|----------|
| Before | 16.69 MB  | 4.23 MB | 3.54 MB |
| After  | 12.07 MB  | 3 MB   | 2.53 MB |
christian-byrne added a commit that referenced this pull request Oct 18, 2025
## Summary

Continuation of #6068:
set `SHOULD_MINIFY` in build workflows to enable minification.

Our build plugin dictates what files are available for extensions to
import at runtime:
https://github.com/Comfy-Org/ComfyUI_frontend/blob/bfe083dcbad61965b0ef04cf91d06a9cddaca361/build/plugins/comfyAPIPlugin.ts#L52

The shimming marks these files and their named exports and named memebrs
as side-effectful, so names should be preserved from the perspective of
3rd party extensions, making this safe in theory.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6116-ci-enable-minification-in-build-workflows-2906d73d36508141be57f63b619f9f16)
by [Unito](https://www.unito.io)
christian-byrne pushed a commit that referenced this pull request Nov 8, 2025
## Summary

Fixes Storybook rendering issue where all components fail to load with
`_sfc_main is not defined` error in **local development** since v1.29.3.

## Problem

After upgrading to v1.29.3, all Storybook components fail to render **in
local development** (`pnpm storybook`) with the following error:

```
ReferenceError: _sfc_main is not defined
The component failed to render properly, likely due to a configuration issue in Storybook.
```

**Important**: This issue only affects **local development**
environments. The deployed/built Storybook works correctly.

This affects both:
- Main Storybook (`pnpm storybook`)
- Desktop-ui Storybook instances

## Root Cause

In v1.29.3, commit `64430708e` ("perf: tree shaking and minify #6068")
enabled build optimizations in `vite.config.mts`:

```typescript
// Before (v1.29.2)
rollupOptions: {
  treeshake: false
}
esbuild: {
  minifyIdentifiers: false
}

// After (v1.29.3)
rollupOptions: {
  treeshake: true  // ⚠️ Enabled
}
esbuild: {
  minifyIdentifiers: SHOULD_MINIFY  // ⚠️ Conditionally enabled
}
```

While these optimizations are beneficial for production builds, they
cause issues in **Storybook's local dev server**:

1. **Tree-shaking in dev mode**: Rollup incorrectly identifies Vue SFC's
`_sfc_main` exports as unused code during the dev server's module
transformation
2. **Identifier minification**: esbuild minifies `_sfc_main` to shorter
names in development, breaking Storybook's HMR (Hot Module Replacement)
and dynamic module loading

Since Storybook's `main.ts` inherits settings from `vite.config.mts` via
`mergeConfig`, these optimizations were applied to Storybook's dev
server configuration, causing Vue components to fail rendering in local
development.

**Why deployed Storybook works**: Production builds have different
optimization pipelines that handle Vue SFCs correctly, but the dev
server's real-time transformation breaks with these settings.

## Solution

Added explicit build configuration overrides in both Storybook
configurations to ensure the **dev server** doesn't inherit problematic
optimizations:

**Files changed:**
- `.storybook/main.ts`
- `apps/desktop-ui/.storybook/main.ts`

**Changes:**
```typescript
esbuild: {
  // Prevent minification of identifiers to preserve _sfc_main in dev mode
  minifyIdentifiers: false,
  keepNames: true
},
build: {
  rollupOptions: {
    // Disable tree-shaking for Storybook dev server to prevent Vue SFC exports from being removed
    treeshake: false,
    // ... existing onwarn config
  }
}
```

This ensures Storybook's **local development server** prioritizes
stability and debuggability over bundle size optimization, while
production builds continue to benefit from tree-shaking and
minification.

## Testing

1. Cleared Storybook and Vite caches: `rm -rf .storybook/.cache
node_modules/.vite`
2. Started local Storybook dev server with `pnpm storybook`
3. Verified all component stories render correctly without `_sfc_main`
errors
4. Ran `pnpm typecheck` to ensure TypeScript compilation succeeds
5. Tested HMR (Hot Module Replacement) works correctly with component
changes

## Context

- This is a **local development-only** issue; deployed Storybook builds
work fine
- Storybook dev server requires special handling because it dynamically
imports and hot-reloads all stories at runtime
- Vue SFC compilation generates `_sfc_main` as an internal identifier
that must be preserved during dev transformations
- Development tools like Storybook benefit from unoptimized builds for
better debugging, HMR, and stability
- Production builds remain optimized with tree-shaking and minification
enabled

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6635-bugfix-Fix-Storybook-_sfc_main-undefined-error-after-v1-29-3-2a56d73d36508194a25eef56789e5e2b)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants