Skip to content

Core: Don't set process.env.NODE_ENV and process.env.DEV#30651

Merged
valentinpalkovic merged 1 commit intonextfrom
valentin/remove-process-env-settings-from-build-step
Feb 25, 2025
Merged

Core: Don't set process.env.NODE_ENV and process.env.DEV#30651
valentinpalkovic merged 1 commit intonextfrom
valentin/remove-process-env-settings-from-build-step

Conversation

@valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Feb 25, 2025

Closes N/A

What I did

I have removed setting process.env.NODE_ENV to production when building packages since it causes issues of loading, e.g., production builds of React into Storybook while running in dev mode. These settings have caused loading production builds of react-refresh during development, leading to failures in Next.js Webpack projects.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 79.6 MB 79.7 MB 37.5 kB -2.84 0%
initSize 79.6 MB 79.7 MB 37.5 kB -2.84 0%
diffSize 97 B 97 B 0 B - 0%
buildSize 7.29 MB 7.29 MB 392 B -1.91 0%
buildSbAddonsSize 1.9 MB 1.9 MB 0 B - 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.88 MB 1.88 MB 0 B 1.11 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.97 MB 3.97 MB 0 B 1.11 0%
buildPreviewSize 3.32 MB 3.32 MB 392 B -1.96 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 7.4s 7.4s 24ms -0.77 0.3%
generateTime 19.5s 22s 2.4s 0.88 11.3%
initTime 5.4s 4.6s -799ms -0.39 -17.1%
buildTime 8.6s 9.5s 909ms 0.04 9.6%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5.5s 4.6s -915ms -0.96 -19.9%
devManagerResponsive 5.3s 4.4s -893ms -0.96 -20.2%
devManagerHeaderVisible 916ms 651ms -265ms -1.55 🔰-40.7%
devManagerIndexVisible 925ms 684ms -241ms -1.59 🔰-35.2%
devStoryVisibleUncached 2.2s 1.7s -538ms -0.28 -30.8%
devStoryVisible 1s 710ms -299ms -1.75 🔰-42.1%
devAutodocsVisible 866ms 690ms -176ms -0.97 -25.5%
devMDXVisible 794ms 731ms -63ms -0.09 -8.6%
buildManagerHeaderVisible 696ms 614ms -82ms -1.12 -13.4%
buildManagerIndexVisible 709ms 691ms -18ms -0.81 -2.6%
buildStoryVisible 684ms 602ms -82ms -1.05 -13.6%
buildAutodocsVisible 589ms 505ms -84ms -1.11 -16.6%
buildMDXVisible 601ms 489ms -112ms -1.2 -22.9%

Greptile Summary

This PR removes the setting of process.env.NODE_ENV and process.env.DEV during the build step to prevent issues with loading production builds in development mode.

  • Removed environment variable settings from the node esbuild options in code/core/scripts/prep.ts
  • Maintains process.env.NODE_ENV settings for browser and finals build configurations
  • Fixes issues with loading production builds of React in development mode
  • Addresses failures in Next.js Webpack projects caused by production builds of react-refresh loading during development

@nx-cloud
Copy link

nx-cloud bot commented Feb 25, 2025

View your CI Pipeline Execution ↗ for commit a664b92.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 2m 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-25 08:50:18 UTC

@storybook-app-bot
Copy link

Package Benchmarks

Commit: a664b92, ran on 25 February 2025 at 08:55:09 UTC

The following packages have significant changes to their size or dependencies:

@storybook/core

Before After Difference
Dependency count 52 52 0
Self size 19.26 MB 19.34 MB 🚨 +74 KB 🚨
Dependency size 14.26 MB 14.26 MB 0 B
Bundle Size Analyzer Link Link

storybook

Before After Difference
Dependency count 53 53 0
Self size 23 KB 23 KB 0 B
Dependency size 33.52 MB 33.59 MB 🚨 +74 KB 🚨
Bundle Size Analyzer Link Link

sb

Before After Difference
Dependency count 54 54 0
Self size 1 KB 1 KB 0 B
Dependency size 33.54 MB 33.62 MB 🚨 +74 KB 🚨
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 358 358 0
Self size 280 KB 280 KB 0 B
Dependency size 83.94 MB 84.02 MB 🚨 +75 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 275 275 0
Self size 612 KB 612 KB 0 B
Dependency size 65.59 MB 65.66 MB 🚨 +74 KB 🚨
Bundle Size Analyzer Link Link

@valentinpalkovic valentinpalkovic marked this pull request as ready for review February 25, 2025 09:06
@valentinpalkovic valentinpalkovic merged commit b325e16 into next Feb 25, 2025
66 of 69 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/remove-process-env-settings-from-build-step branch February 25, 2025 09:06
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@github-actions github-actions bot mentioned this pull request Feb 25, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant