Skip to content

Import useEffect normally#5501

Merged
adhami3310 merged 1 commit into
mainfrom
masenf/do-not-reference-React
Jun 26, 2025
Merged

Import useEffect normally#5501
adhami3310 merged 1 commit into
mainfrom
masenf/do-not-reference-React

Conversation

@masenf
Copy link
Copy Markdown
Collaborator

@masenf masenf commented Jun 26, 2025

The Layout isn't wrapped in an ErrorBoundary, and this particular usage of useEffect seems to trip up HMR in the sandbox when replacing the app code. Instead, use a direct named import, which might be handled more nicely in vite.

The `Layout` isn't wrapped in an ErrorBoundary, and this particular usage of
`useEffect` seems to trip up HMR in the sandbox when replacing the app code.
Instead, use a direct named import, which might be handled more nicely in vite.
Copy link
Copy Markdown
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.

PR Summary

Modified React useEffect import strategy to improve Hot Module Replacement (HMR) behavior in the development sandbox environment.

  • Changed from React.useEffect to direct named import in reflex/.templates/jinja/web/pages/_app.js.jinja2
  • Updated compiler imports in reflex/compiler/compiler.py to include useEffect as a named import alongside Fragment
  • Potential Issue: useEffect is used but not explicitly imported in _app.js.jinja2, which needs to be addressed

2 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile


export function Layout({children}) {
React.useEffect(() => {
useEffect(() => {
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.

logic: useEffect is used but not imported. Add import { useEffect } from 'react'; to the declaration block

Suggested change
useEffect(() => {
import { useEffect } from 'react';
useEffect(() => {

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 26, 2025

CodSpeed Performance Report

Merging #5501 will not alter performance

Comparing masenf/do-not-reference-React (6284d09) with main (e3cc02d)

Summary

✅ 8 untouched benchmarks

@adhami3310 adhami3310 merged commit b816328 into main Jun 26, 2025
41 checks passed
@adhami3310 adhami3310 deleted the masenf/do-not-reference-React branch June 26, 2025 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants