Skip to content
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

chore: update react-scripts to v5 #126

Merged
merged 4 commits into from
Apr 4, 2022
Merged

Conversation

ericrallen
Copy link
Contributor

@ericrallen ericrallen commented Mar 30, 2022

Motivation

This updates react-scripts in the generated starter app from @datadog/create-app in order to resolve the react-error-overlay issue that was preventing hot reloading from working while developing an app after using yarn create @datadog/app.

References:

Changes

Upgrade to v5.0.0 of react-scripts

Testing

To See the Current Issue

  1. Clear yarn cache and run yarn create @datadog/app
  2. Run yarn start in generated app
  3. Make change to src/widget/index.tsx in generated app
  4. Notice that hot reload does not work

To Test the Idea Behind This Fix

  1. Delete node_modules and yarn.lock
  2. Update react-scripts version in package.json to ^5.0.0
  3. Run yarn
  4. Repeat steps 2 & 3 from the To See the Current Issue above
  5. Notice that hot reload does work

To Test this Fix:

  1. Checkout this branch
  2. yarn link the @datadog/create-app package
  3. Run yarn create @datadog/app --commit 52f5e05d00077915e8a7ed424d4bd83ae6e1fe85
  4. Run yarn start in generated app
  5. Make a change to src/widget/index.tsx in generated app
  6. Notice that hot reload does work
  7. Shut down the development server
  8. yarn unlink the @datadog/create-app package

Releases

Choose one:

  • No release is necessary.
    If you're only updating examples/documentation, this is likely what you want.
  • All packages that need a release have a changeset
    Only @datadog/create-app needs a release

@ericrallen ericrallen added bug Something isn't working dependencies Pull requests that update a dependency file labels Mar 30, 2022
@ericrallen ericrallen requested a review from joneshf-dd March 30, 2022 19:17
@ericrallen
Copy link
Contributor Author

Here's s screenshot of the previous behavior where the react-error-overlay <iframe> was blocking the DOM and preventing user interaction, too:

Screen Shot 2022-03-30 at 10 24 45

Copy link
Collaborator

@joneshf-dd joneshf-dd left a comment

Choose a reason for hiding this comment

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

Sorry this broke on you. But, thanks for looking into it!

There's a bit more work before we can merge it. I tried to explain the situation and some ideas of what we can do, totally up to you what you'd like here.

packages/create-app/src/template/package.json Outdated Show resolved Hide resolved
@ericrallen ericrallen force-pushed the feature/update-react-scripts branch from a2527d4 to 52f5e05 Compare March 30, 2022 22:02
@ericrallen ericrallen requested a review from joneshf-dd March 30, 2022 22:37
Eric allen added 2 commits April 4, 2022 09:56
This updates react-scripts in the various starter apps in order to resolve an issue with hot reloading while working locally.

References:
- facebook/create-react-app#11771
- https://stackoverflow.com/a/70452191/656011
@ericrallen ericrallen force-pushed the feature/update-react-scripts branch from a9bc44f to c332933 Compare April 4, 2022 14:35
@ericrallen ericrallen force-pushed the feature/update-react-scripts branch from c332933 to fa98d59 Compare April 4, 2022 15:23
Copy link
Collaborator

@joneshf-dd joneshf-dd left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for going through this.

One last bit about bumping the packages. But as mentioned, if you'd prefer to avoid making more changes/dealing with versions, just merge it now and we'll follow-up on the version bumps before making the next release.

packages/ui-extensions-react/package.json Outdated Show resolved Hide resolved
.changeset/cuddly-pugs-hammer.md Show resolved Hide resolved
@ericrallen ericrallen force-pushed the feature/update-react-scripts branch from debad6e to 517494b Compare April 4, 2022 18:08
Copy link
Collaborator

@joneshf-dd joneshf-dd left a comment

Choose a reason for hiding this comment

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

Thanks again for doing all this!

@ericrallen ericrallen merged commit 58a9034 into master Apr 4, 2022
@ericrallen ericrallen deleted the feature/update-react-scripts branch April 4, 2022 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants