Skip to content

Conversation

@fluiddot
Copy link
Contributor

Proposed Changes

  • Install React Devtools to enable inspecting components and profiling the performance of the app.
Captura de pantalla 2024-09-23 a las 13 57 30

Note

The React Devtools don't show up when starting the app due to a known bug (MarshallOfSound/electron-devtools-installer#244). They appear when making a change in the code (i.e. when hot-reloading is triggered) or by reloading manually the window.

Testing Instructions

  • Start the app with the command npm start.
  • Observe the Devtools window is shown.
  • Focus on the Studio app and reload the window with Command-R or menu item View->Reload.
  • Observe the React Components and Profile tabs show up in the Devtools.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@fluiddot fluiddot requested a review from a team September 23, 2024 11:58
@fluiddot fluiddot self-assigned this Sep 23, 2024
Copy link
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

The changes worked as expected to me. I was able to see the React tools activated.

One small note on the testing instructions is that I had to do Force Reload instead of just Reload to make it work.

I remember that some time ago, we also introduced standalone React dev tools in https://github.com/Automattic/local-environment/pull/148 - do we need both?

@fluiddot
Copy link
Contributor Author

One small note on the testing instructions is that I had to do Force Reload instead of just Reload to make it work.

Ah, in my case just reloading worked but it might depend on the state of the app. I'll update the testing instructions, thanks!

I remember that some time ago, we also introduced standalone React dev tools in Automattic/local-environment#148 - do we need both?

Oh, I overlooked this documentation when I tried to use the React Devtools. I think we can have both as they provide two different ways to access React Devtools:

Ideally, the latter could supersede the former if we find a workaround to avoid reloading the window to make the React Devtools visible. In any case, I'm ok if we rather keep one option and discard this approach 🙂.

@katinthehatsite
Copy link
Contributor

Personally, I prefer approach in this PR to the one in: https://github.com/Automattic/local-environment/pull/148 because that one requires you to start extra things while this approach is more straightforward 👍 I also do not mind reloading the window and I think it is okay for now as long as we mention it explicitly in the documentation.

That is to say that I think we definitely should keep this approach. As for the standalone tools, we can either keep them or remove them - I do not have a strong preference. I do not use is very often and with this new approach will likely use them even less often.

@fluiddot
Copy link
Contributor Author

Personally, I prefer approach in this PR to the one in: Automattic/local-environment#148 because that one requires you to start extra things while this approach is more straightforward 👍 I also do not mind reloading the window and I think it is okay for now as long as we mention it explicitly in the documentation.

Good point. I'll update the documentation in a separate PR to mention this.

That is to say that I think we definitely should keep this approach. As for the standalone tools, we can either keep them or remove them - I do not have a strong preference. I do not use is very often and with this new approach will likely use them even less often.

Sounds good. I also don't have a strong preference for either keeping it or removing it. Since both can live together, I'd lean toward keeping both for now.

@fluiddot fluiddot merged commit 2bf06f1 into trunk Sep 24, 2024
@fluiddot fluiddot deleted the add/enable-react-devtools branch September 24, 2024 08:23
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.

3 participants