feat: add integration with Redux DevTools #289

pac-guerreiro · 2023-07-27T09:53:02Z

Details

This PR adds the ability to use React Native Onyx with Redux DevTools to debug storage changes.

Features:

View SET, MULTISET, MERGE, REMOVE and CLEAR actions in DevTools: payload data corresponds to the data which was written into the storage;
View state diffs when an action is selected;
View current state at the time an action was called;
Time travelling is not possible (yet)

Related Issues

None

Automated Tests

Not needed because this doesn't affect internal behaviour

Linked PRs

None

github-actions · 2023-07-27T09:53:16Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

pac-guerreiro · 2023-07-27T09:53:45Z

I have read the CLA Document and I hereby sign the CLA

AndrewGable · 2023-07-27T16:36:42Z

Can you please add some documentation on how to get this setup as a developer?

pac-guerreiro · 2023-07-28T16:47:39Z

@AndrewGable

I just added some instructions to the README.md and also updated the API.md.

I decided to differentiate between debugging setState and using Redux DevTools extension.

Let me know what you think! 😄

PauloGasparSv · 2023-07-31T19:53:29Z

Hey @pac-guerreiro we have a conflict now, can you please take a look?

I'll review this tomorrow, got back from OOO today and had to prioritize other issues/reviews.

AndrewGable · 2023-08-01T02:51:19Z

README.md

+
+First, install Redux DevTools through your favorite browser ([Edge](https://microsoftedge.microsoft.com/addons/detail/redux-devtools/nnkgneoiohoecpdiaponcejilbhhikei), [Chrome](https://chrome.google.com/webstore/detail/redux-devtools/lmhkpmbekcpmknklioeibfkpmmfibljd), [Firefox](https://addons.mozilla.org/en-US/firefox/addon/reduxdevtools/))
+
+Then, you can enable this type of debugging by passing `enableDevTools: true` to `Onyx.init({...})`.


I kinda wonder if this should be enabled by default on dev?

@AndrewGable I'm not sure if it won't be impactful on performance 😅

pac-guerreiro · 2023-08-01T15:16:34Z

Hey @pac-guerreiro we have a conflict now, can you please take a look?

I'll review this tomorrow, got back from OOO today and had to prioritize other issues/reviews.

@PauloGasparSv I just resolved the conflict 😄

PauloGasparSv · 2023-08-02T22:45:05Z

Sorry for the delay, I had time to check the changes here (so far lgtm) but didn't test this yet. Will test it using the extension tomorrow : )

pac-guerreiro · 2023-08-03T10:37:52Z

@PauloGasparSv

No problem 😄 I just fixed the tests that were broken because of my changes!

PauloGasparSv · 2023-08-03T14:05:16Z

lib/DevTools.js

@@ -0,0 +1,54 @@
+import {connectViaExtension, extractState} from 'remotedev';


Hey @pac-guerreiro I'm really curious on how you decided on using remotedev! I'm relatively new to React/ReactNative so would love to know : D

I could only find this slack post you sent recently on this.

(btw, this is super cool)
(btw 2, had to shift focus but I'm still planning on testing today)

@PauloGasparSv

Sorry for the late response, I'm glad you like this idea 😄 I hope it helps a lot of developers working on the app 😄

Well, at first I thought I could fork the repo for redux dev tools extension but then I found it was too complex to adapt it as it was for Onyx, so after some digging and research I found remotedev, which was made by the same guy behind redux dev tools extension, if I'm not mistaken. It basically provides an api to connect to redux dev tools or any other extension that supports remotedev, so I gave it a shot and it worked 😄

PauloGasparSv

LGTM, this is pretty neat!

All yours @AndrewGable @Gonals

Gonals · 2023-08-08T13:17:58Z

OOO starting tomorrow, so removing myself!

PauloGasparSv · 2023-08-08T14:22:16Z

@pac-guerreiro do you mind fixing the new conflicts we have?

Bump @AndrewGable for a review : D

pac-guerreiro · 2023-08-09T09:19:04Z

@PauloGasparSv

It is done 😄

PauloGasparSv · 2023-08-09T14:31:39Z

Hey @pac-guerreiro I think I may have found a bug in this line this time (didn't happen before, not sure why it happened this time):

I think the initial state can be undefined:

I think this error prevented me from signing because after I reinstalled dependencies the error was gone and I could sign in as usual.

Can you please take a look at that?
LMK if you can't replicate, maybe there is a problem in my environment :D

pac-guerreiro · 2023-08-09T15:12:58Z

@PauloGasparSv

This issue happens inside remotedev's extractState function, in which JSON.parse returns this state as undefined sometimes 😅

Honestly, I guess the whole callback passed to DevTools constructor can be discarded since this was meant to update the storage each time we time travel between actions in DevTools but it doesn't work correctly 😅

PauloGasparSv · 2023-08-09T16:37:45Z

This issue happens inside remotedev's extractState function, in which JSON.parse returns this state as undefined sometimes 😅

Oh I see, I didn't double check but does it return undefined even if there is state data there?

If that's the case then it doesn't even make sense to coalesce that value but maybe we can skip that logic that removes the keys in case newState is undefined

Honestly, I guess the whole callback passed to DevTools constructor can be discarded since this was meant to update the storage each time we time travel between actions in DevTools but it doesn't work correctly 😅

Really?? So we don't that need that bit of logic?

Sorry for the confusion ahaha I'm trying to understand what we can do next here : )

pac-guerreiro · 2023-08-10T14:42:42Z

@PauloGasparSv

Oh I see, I didn't double check but does it return undefined even if there is state data there?

Yeah, I think it has some trouble parsing slash tokens sometimes, maybe because the extension is sending some messages in a format that remotedev is not ready to interpret 😅

Really?? So we don't that need that bit of logic?

It could be useful for debugging purposes if a dev wants to go back in time and see the state changes in real-time, but since this logic is broken because either that undefined issue occurs or it does work but going back causes new events to get triggered and shown in the dev tools 😅

So yeah, if you think it's better, I can drop this logic and we just provide the ability to check state changes in real time 😄

PauloGasparSv · 2023-08-10T14:45:01Z

So yeah, if you think it's better, I can drop this logic and we just provide the ability to check state changes in real time 😄

That sounds awesome to me! @AndrewGable do you mind giving your opinion on this too?

PauloGasparSv · 2023-08-14T13:09:09Z

Bump @AndrewGable : )

AndrewGable · 2023-08-14T16:26:25Z

Yes that sounds good 👍

PauloGasparSv · 2023-08-17T13:19:00Z

Bump @pac-guerreiro :D

TomaszFrackowiak · 2023-08-18T10:49:01Z

@PauloGasparSv - hey, just to let you know that @pac-guerreiro is ooo till 25.08

PauloGasparSv · 2023-08-18T12:47:31Z

@PauloGasparSv - hey, just to let you know that @pac-guerreiro is ooo till 25.08

Wasn't aware, thanks a lot @TomaszFrackowiak!!!

pac-guerreiro · 2023-08-28T16:07:40Z

@PauloGasparSv

Sorry for not warning you about my vacation!

I just removed the code that was causing problems and resolved conflicts 😄

pac-guerreiro · 2023-09-04T09:52:40Z

Bump @AndrewGable

AndrewGable · 2023-09-05T19:50:06Z

Oops. Looks like your going to have to re-sign your commits.

feat: add ability to time travel between state changes fix: add missing registerAction for merge fix(DevTools): state updates not being synched between tabs docs(DevTools): add proper documentation

pac-guerreiro · 2023-09-06T19:02:56Z

@AndrewGable it is solved now 😄

ospfranco · 2023-09-11T08:42:15Z

Trying to run the tests on the main Expensify repo and this change causes the tests to fail:

Seems the remotedev dependency was added as a dev dependency which is not installed when ran from the main repo.

Revert "Merge pull request #289 from pac-guerreiro/feature/remotedev"

AndrewGable · 2023-09-19T03:32:36Z

@pac-guerreiro - Can you address this issue in a PR then re-submit? Looks like this PR was reverted.

pac-guerreiro · 2023-12-21T01:12:35Z

@AndrewGable Hope I'm not late to the party but here's the new PR for this work - #438

I redone everything from scratch since some things got changed in Onyx during this period.

Instead of relying on third party node modules, I decided to implement the logic to connect to the extension by myself after doing some research into remotedev repo.

Onyx will connect to Redux DevTools extension by default.

Developers will only be able to use this functionality on web apps.

pac-guerreiro requested a review from a team as a code owner July 27, 2023 09:53

melvin-bot bot requested review from Gonals and removed request for a team July 27, 2023 09:53

pac-guerreiro force-pushed the feature/remotedev branch from 76a9453 to acaaeed Compare July 27, 2023 09:57

pac-guerreiro force-pushed the feature/remotedev branch 2 times, most recently from 698a175 to c63c063 Compare July 28, 2023 16:46

Gonals requested a review from a team July 31, 2023 09:31

melvin-bot bot requested review from PauloGasparSv and removed request for a team July 31, 2023 09:32

AndrewGable reviewed Aug 1, 2023

View reviewed changes

pac-guerreiro force-pushed the feature/remotedev branch from c63c063 to b080b88 Compare August 1, 2023 15:15

PauloGasparSv reviewed Aug 3, 2023

View reviewed changes

PauloGasparSv previously approved these changes Aug 7, 2023

View reviewed changes

Gonals removed their request for review August 8, 2023 13:17

AndrewGable previously approved these changes Aug 8, 2023

View reviewed changes

pac-guerreiro dismissed stale reviews from AndrewGable and PauloGasparSv via cda6b1b August 9, 2023 09:18

pac-guerreiro force-pushed the feature/remotedev branch from f687142 to cda6b1b Compare August 9, 2023 09:18

pac-guerreiro force-pushed the feature/remotedev branch from 83357ba to 9139a1f Compare August 28, 2023 16:06

AndrewGable approved these changes Sep 5, 2023

View reviewed changes

Pedro Guerreiro and others added 6 commits September 6, 2023 19:57

feat: add integration with redux devtools extension

15983a8

feat: add ability to time travel between state changes fix: add missing registerAction for merge fix(DevTools): state updates not being synched between tabs docs(DevTools): add proper documentation

refactor(DevTools): remove unnecessary then

97a4815

refactor(DevTools): differentiate debug modes

5483713

docs(DevTools): add explanation on how to use Redux DevTools extension

f26f037

chore: fix broken tests

a62e52b

fix(devtools): time travel causes app to crash

eb2ffc4

pac-guerreiro force-pushed the feature/remotedev branch from 9139a1f to eb2ffc4 Compare September 6, 2023 18:57

AndrewGable merged commit 35ef09a into Expensify:main Sep 6, 2023
3 checks passed

ospfranco mentioned this pull request Sep 11, 2023

Update Onyx to use a map for Onyx collection keys Expensify/App#27130

Merged

59 tasks

mountiny added a commit that referenced this pull request Sep 12, 2023

Merge pull request #337 from ospfranco/osp/revert-devtools

3c7448e

Revert "Merge pull request #289 from pac-guerreiro/feature/remotedev"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add integration with Redux DevTools #289

feat: add integration with Redux DevTools #289

pac-guerreiro commented Jul 27, 2023

github-actions bot commented Jul 27, 2023 •

edited

Loading

pac-guerreiro commented Jul 27, 2023

AndrewGable commented Jul 27, 2023

pac-guerreiro commented Jul 28, 2023

PauloGasparSv commented Jul 31, 2023

AndrewGable Aug 1, 2023

pac-guerreiro Aug 1, 2023

pac-guerreiro commented Aug 1, 2023

PauloGasparSv commented Aug 2, 2023

pac-guerreiro commented Aug 3, 2023

PauloGasparSv Aug 3, 2023 •

edited

Loading

pac-guerreiro Aug 5, 2023

PauloGasparSv left a comment •

edited

Loading

Gonals commented Aug 8, 2023

PauloGasparSv commented Aug 8, 2023

pac-guerreiro commented Aug 9, 2023

PauloGasparSv commented Aug 9, 2023 •

edited

Loading

pac-guerreiro commented Aug 9, 2023

PauloGasparSv commented Aug 9, 2023

pac-guerreiro commented Aug 10, 2023

PauloGasparSv commented Aug 10, 2023

PauloGasparSv commented Aug 14, 2023

AndrewGable commented Aug 14, 2023

PauloGasparSv commented Aug 17, 2023

TomaszFrackowiak commented Aug 18, 2023

PauloGasparSv commented Aug 18, 2023

pac-guerreiro commented Aug 28, 2023

pac-guerreiro commented Sep 4, 2023

AndrewGable commented Sep 5, 2023

pac-guerreiro commented Sep 6, 2023

ospfranco commented Sep 11, 2023

AndrewGable commented Sep 19, 2023

pac-guerreiro commented Dec 21, 2023


		First, install Redux DevTools through your favorite browser ([Edge](https://microsoftedge.microsoft.com/addons/detail/redux-devtools/nnkgneoiohoecpdiaponcejilbhhikei), [Chrome](https://chrome.google.com/webstore/detail/redux-devtools/lmhkpmbekcpmknklioeibfkpmmfibljd), [Firefox](https://addons.mozilla.org/en-US/firefox/addon/reduxdevtools/))

		Then, you can enable this type of debugging by passing `enableDevTools: true` to `Onyx.init({...})`.

		@@ -0,0 +1,54 @@
		import {connectViaExtension, extractState} from 'remotedev';

feat: add integration with Redux DevTools #289

feat: add integration with Redux DevTools #289

Conversation

pac-guerreiro commented Jul 27, 2023

Details

Related Issues

Automated Tests

Linked PRs

github-actions bot commented Jul 27, 2023 • edited Loading

pac-guerreiro commented Jul 27, 2023

AndrewGable commented Jul 27, 2023

pac-guerreiro commented Jul 28, 2023

PauloGasparSv commented Jul 31, 2023

AndrewGable Aug 1, 2023

Choose a reason for hiding this comment

pac-guerreiro Aug 1, 2023

Choose a reason for hiding this comment

pac-guerreiro commented Aug 1, 2023

PauloGasparSv commented Aug 2, 2023

pac-guerreiro commented Aug 3, 2023

PauloGasparSv Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

pac-guerreiro Aug 5, 2023

Choose a reason for hiding this comment

PauloGasparSv left a comment • edited Loading

Choose a reason for hiding this comment

Gonals commented Aug 8, 2023

PauloGasparSv commented Aug 8, 2023

pac-guerreiro commented Aug 9, 2023

PauloGasparSv commented Aug 9, 2023 • edited Loading

pac-guerreiro commented Aug 9, 2023

PauloGasparSv commented Aug 9, 2023

pac-guerreiro commented Aug 10, 2023

PauloGasparSv commented Aug 10, 2023

PauloGasparSv commented Aug 14, 2023

AndrewGable commented Aug 14, 2023

PauloGasparSv commented Aug 17, 2023

TomaszFrackowiak commented Aug 18, 2023

PauloGasparSv commented Aug 18, 2023

pac-guerreiro commented Aug 28, 2023

pac-guerreiro commented Sep 4, 2023

AndrewGable commented Sep 5, 2023

pac-guerreiro commented Sep 6, 2023

ospfranco commented Sep 11, 2023

AndrewGable commented Sep 19, 2023

pac-guerreiro commented Dec 21, 2023

github-actions bot commented Jul 27, 2023 •

edited

Loading

PauloGasparSv Aug 3, 2023 •

edited

Loading

PauloGasparSv left a comment •

edited

Loading

PauloGasparSv commented Aug 9, 2023 •

edited

Loading