Skip to content

fix: avoid npm reinstall for empty devDependencies - #7566

Open
harsh21234i wants to merge 4 commits into
reflex-dev:mainfrom
harsh21234i:fix/7480-npm-manifest-normalization
Open

harsh21234i wants to merge 4 commits into
reflex-dev:mainfrom
harsh21234i:fix/7480-npm-manifest-normalization

Conversation

@harsh21234i

@harsh21234i harsh21234i commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Treat a missing devDependencies field and an empty object as equivalent when comparing rendered package manifests.
  • Preserve the npm install cache when a source-only reload produces no meaningful manifest change.
  • Add regression tests for manifest sync and install-cache preservation.

Fixes #7480

Tests

  • uv run pytest tests/units/utils/test_frontend_skeleton.py tests/units/utils/test_js_runtimes.py -q (24 passed)
  • Targeted pre-commit checks passed.
  • Verified locally with REFLEX_USE_NPM=1: a Python source-only hot reload rebuilt the app without rerunning npm install.

View guided diff

@harsh21234i
harsh21234i requested a review from a team as a code owner October 14, 2026 17:20

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 3 files

View guided diff | Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Treats missing and empty devDependencies as equivalent so source-only reloads avoid unnecessary npm installs.

  • An empty devDependencies field no longer triggers a frontend reinstall.

Reviews (4) · Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." · Reviewed by Greptile

Comment thread reflex/utils/frontend_skeleton.py
Comment thread reflex/utils/frontend_skeleton.py Outdated
Comment thread tests/units/utils/test_frontend_skeleton.py Outdated
@codspeed

codspeed Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 4.12%

⚠️ 17 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 147 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_route_argument_extraction 119.7 µs 114.9 µs +4.12%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing harsh21234i:fix/7480-npm-manifest-normalization (f35ef79) with main (7ccaa4e)

Open in CodSpeed

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant