Skip to content

Fix error & injection of iframe on hot reload #2214

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 1 commit into from
Sep 14, 2022

Conversation

ianyong
Copy link
Member

@ianyong ianyong commented Sep 12, 2022

Description

Fixes #2213.

Same issue as facebook/create-react-app#11773.

There are two solutions outlined in the CRA issue above:

  1. Upgrade react-scripts to v5.x.
  2. Specify v6.0.9 of react-error-overlay via selective dependency resolution.

Unfortunately, we cannot upgrade react-scripts (CRA) to v5.0x because another dependency that we use, Create React App Configuration Override (CRACO) does not have a stable version that supports CRA v5.x yet:

ℹ️ The latest CRACO release does not support CRA 5, but alpha builds will be regularly released over the next few days.

-- https://github.com/dilanx/craco (as of 13 September 2022)

Attempting to do so results in errors when starting the frontend. As such, we have no choice but to go with the second option for now.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

  1. Launch the frontend with developer tools open.
  2. Make a change in the codebase to trigger a hot reload.
  3. Check that the error described in the issue was not thrown, and that no iframe was injected.

@ianyong ianyong requested a review from zhaojj2209 September 12, 2022 16:17
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3038875426

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 33.653%

Totals Coverage Status
Change from base Build 3009124676: 0.0%
Covered Lines: 4281
Relevant Lines: 11885

💛 - Coveralls

Copy link
Contributor

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

Hot reload works now, thanks for the fix!

@ianyong ianyong merged commit a78d5cc into master Sep 14, 2022
@ianyong ianyong deleted the fix-error-on-hot-reload branch September 14, 2022 14:48
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.

[Development Only] Error thrown & iframe injected when the application is hot reloaded
3 participants