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

refactor: move getCurrentChainId from selectors/selectors.js to shared/modules/selectors/networks.ts #27647

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

davidmurdoch
Copy link
Contributor

@davidmurdoch davidmurdoch commented Oct 4, 2024

Converts the selector getCurrentChainId from functions from JS to TS and moves it to the shared folder. Also updates some functions to match the actual expect return values and fixes some types.

Why only this function? I'm trying to solve circular dependency issues. getCurrentChainId is so widely used in the codebase, it makes it very complicated to untangle. I've moved it to shared/modules/selectors/networks.ts

Copy link
Contributor

github-actions bot commented Oct 4, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@davidmurdoch davidmurdoch added the team-tiger Tiger team (for tech debt reduction + performance improvements) label Oct 9, 2024
@davidmurdoch davidmurdoch changed the title move getCurrentChainId from selectors/selectors.js to `selectors/… move getCurrentChainId from selectors/selectors.js to selectors/network.ts Oct 9, 2024
@davidmurdoch davidmurdoch changed the title move getCurrentChainId from selectors/selectors.js to selectors/network.ts chore: move getCurrentChainId from selectors/selectors.js to selectors/network.ts Oct 9, 2024
@davidmurdoch davidmurdoch changed the title chore: move getCurrentChainId from selectors/selectors.js to selectors/network.ts refactor: move getCurrentChainId from selectors/selectors.js to selectors/network.ts Oct 9, 2024
@davidmurdoch davidmurdoch force-pushed the circles-008 branch 4 times, most recently from 2341b7f to 6f8db43 Compare October 9, 2024 19:11
@davidmurdoch davidmurdoch force-pushed the circles-008 branch 2 times, most recently from 6737f10 to 0c6f2a7 Compare October 16, 2024 22:32
Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [209dd01]
Page Load Metrics (2031 ± 101 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint170225712028215103
domContentLoaded165025541994210101
load169925702031211101
domInteractive15104472412
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -110 Bytes (-0.00%)
  • ui: -94 Bytes (-0.00%)
  • common: 4.21 KiB (0.06%)

@davidmurdoch davidmurdoch marked this pull request as ready for review October 18, 2024 14:46
@davidmurdoch davidmurdoch requested review from a team as code owners October 18, 2024 14:46
@davidmurdoch davidmurdoch requested review from a team as code owners October 30, 2024 20:30
@davidmurdoch davidmurdoch force-pushed the circles-008 branch 2 times, most recently from 9a02a40 to bd54012 Compare November 4, 2024 16:47
shane-t
shane-t previously approved these changes Nov 27, 2024
@davidmurdoch davidmurdoch added this pull request to the merge queue Nov 27, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 27, 2024
@HowardBraham HowardBraham added this pull request to the merge queue Nov 27, 2024
@HowardBraham HowardBraham removed this pull request from the merge queue due to a manual request Nov 27, 2024
@danjm danjm merged commit 85baff3 into develop Nov 27, 2024
74 of 75 checks passed
@danjm danjm deleted the circles-009 branch November 27, 2024 15:43
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2024
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 27, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [d640044]
Page Load Metrics (1658 ± 70 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14532100166614871
domContentLoaded14382036163714067
load14502103165814570
domInteractive22101452311
backgroundConnect96625188
firstReactRender1573352412
getState448994
initialActions00000
loadScripts10501538123111354
setupStore65614168
uiStartup15972328183816880
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -5 Bytes (-0.00%)
  • ui: 5.68 KiB (0.07%)
  • common: 969 Bytes (0.01%)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-extension-platform team-tiger Tiger team (for tech debt reduction + performance improvements)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants