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

[react devtools][easy] Change function names and remove unused functions, add constants, etc #25211

Merged
merged 1 commit into from
Sep 9, 2022
Merged

[react devtools][easy] Change function names and remove unused functions, add constants, etc #25211

merged 1 commit into from
Sep 9, 2022

Conversation

rbalicki2
Copy link
Contributor

@rbalicki2 rbalicki2 commented Sep 8, 2022

Summary

  • Minor cleanup: Change variable names, put stuff in constants, etc. in preparation for next diff
  • Remove functions that are unused (from utils) + remove setShowInlineWarningsAndErrors, which is only called in setupTests and is unnecessary.
  • Add type safety to getXXX: boolean() functions, which will no longer return JSON.parse(...). Instead, we check for the strings true and false.

How did you test this change?

  • yarn prettier, yarn run test-build-devtools, yarn run flow dom

@rbalicki2 rbalicki2 requested a review from lunaruan September 8, 2022 16:27
@rbalicki2 rbalicki2 changed the title [react devtools][easy] Change variable names, etc. [react devtools][easy] Change function names and remove unused functions, add constants, etc Sep 8, 2022
* Change variable names, put stuff in constants, etc. in preparation for next diff
);
return parseBool(raw) ?? true;
Copy link
Contributor

@lunaruan lunaruan Sep 8, 2022

Choose a reason for hiding this comment

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

we probably don't need the parseBool method (might create unnecessary indirection that might make the code more confusing) What do you think about removing parseBool and replacing it with something like:

return typeof raw === 'string' && raw === 'false' ? false : true;

return true;
}

export function setShowInlineWarningsAndErrors(value: boolean): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you remove this you can probably also remove the other setters since they're also not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I removed all the unused setters 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only one of the setters was used, and it was used in setupTests (but didn't actually affect the output of any tests)

@rbalicki2 rbalicki2 merged commit 540ba5b into facebook:main Sep 9, 2022
sammy-SC pushed a commit that referenced this pull request Sep 11, 2022
* Change variable names, put stuff in constants, etc. in preparation for next diff
rickhanlonii pushed a commit that referenced this pull request Oct 5, 2022
* Change variable names, put stuff in constants, etc. in preparation for next diff
rickhanlonii pushed a commit that referenced this pull request Oct 5, 2022
* Change variable names, put stuff in constants, etc. in preparation for next diff
GrinZero added a commit to GrinZero/react that referenced this pull request Nov 7, 2022
* 'main' of ssh://github.com/GrinZero/react: (51 commits)
  Flow: add simple explicit export types to Devtools (facebook#25251)
  [react devtools][easy] Centralize calls to patchConsoleUsingWindowValues (facebook#25222)
  Unwind the current workInProgress if it's suspended (facebook#25247)
  Add early exit to strict mode (facebook#25235)
  fix: prettier ignore removed and fixed (facebook#24811)
  Flow: enable unsafe-addition error (facebook#25242)
  Flow: upgrade to 0.132 (facebook#25244)
  Flow: fix Fiber typed as any (facebook#25241)
  Flow: ReactFiberHotReloading recursive type (facebook#25225)
  Add some test coverage for some error cases (facebook#25240)
  experimental_use(context) for server components and ssr (facebook#25226)
  Flow: upgrade to 0.131 (facebook#25224)
  Prevent infinite re-renders in StrictMode + Offscreen (facebook#25203)
  Flow: remove explicit object syntax (facebook#25223)
  Flow: upgrade to 0.127 (facebook#25221)
  Flow: enable exact_by_default (facebook#25220)
  [react devtools] Don't check for NODE_ENV==='test' because it never is (facebook#25186)
  [react devtools][easy] Change variable names, etc. (facebook#25211)
  Bump async from 2.6.3 to 2.6.4 in /fixtures/concurrent/time-slicing (facebook#24443)
  Flow: implicit-inexact-object=error (facebook#25210)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants