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

Import-Export States to your DevTools #118

Closed

Conversation

pabloalonsos
Copy link

I've put together a small addition to the code that would allow us to import and export action logs.

  • Export button added to top bar.
  • Copy the current state from the textarea and paste it on another session running the app.

This feature (or something similar) would be awesome to use for QA purposes among other things.

@SimenB
Copy link

SimenB commented Sep 26, 2015

Huge 👍 Anything keeping this back? @gaearon seemed keen on it in #117

@gaearon
Copy link
Contributor

gaearon commented Sep 26, 2015

Only that we're splitting LogMonitor into a separate project: #132. Once that's done, I'll definitely take a look. Sorry it's taking long!

@SimenB
Copy link

SimenB commented Sep 26, 2015

Haha, I love how you define this as a long wait, when it's been a week. I was just wondering if it had fallen through the cracks 😄 No problem there though, apparently. Keep up the great work!

Btw (and sorta related), is there any possibility of getting a sync between browsers/sessions? So you can have different browsers up at the same time all getting the same data automatically. I sorta have it now using browsersync, but that syncs clicks and inputs, and not actual state, which would be nicer, IMO. If it's available, or discussed in another issue, apologies.
I was watching a talk on Figwheel, and they have it (called it broadcasting, IIRC)

@gaearon
Copy link
Contributor

gaearon commented Sep 26, 2015

Btw (and sorta related), is there any possibility of getting a sync between browsers/sessions?

You can implement this as a store enhancer for regular Redux—it will work with Redux DevTools just fine. You can use persistState as inspiration (though it does a different thing).

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2015

@pabloalonsos

Can I ask you to tweak this to:

  • Have separate Import and Export buttons on the toolbar (never mind the design; we'll fix it later)
  • Remove the notion of "export mode"
  • Instead of a text area, show JSON in window.prompt for both import and export

?

break;
case ActionTypes.IMPORT:
(
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove extra indentation and line break here.
This is fine:

({
  stagedActions,
  ...
} = liftedAction.newState);

Let's also rename newState to nextLiftedState.

@gaearon gaearon mentioned this pull request Sep 27, 2015
@pabloalonsos
Copy link
Author

sounds good! I'll try to make the changes today 👍

@pabloalonsos
Copy link
Author

There you go. Let me know if all is good and I can squash those commits into one.

There are a couple of "issues" (not really issues but worth bringing up) I ran into:

  • By using window.prompt instead of the textarea component, ESLint is throwing a couple of warnings. I don't mind finding a different approach if we want to get rid of those warnings without modifying the lint rules (using a custom modal component with a textarea or something like that).
  • ESLint was also complaining about having import as an action (because of the reserved word) so I switched it to importState. I'm pretty bad at picking names for attributes and variables so feel free to suggest alternatives if those are not the best names for this.

Thanks guys!

@@ -3,6 +3,7 @@ const ActionTypes = {
RESET: 'RESET',
ROLLBACK: 'ROLLBACK',
COMMIT: 'COMMIT',
IMPORT: 'IMPORT',
Copy link
Contributor

Choose a reason for hiding this comment

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

let's call it IMPORT_STATE yup

@pabloalonsos
Copy link
Author

Updated 👍

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2015

One more thing: can you please add some tests?

@pabloalonsos
Copy link
Author

Busy couple of days at work. I'll try and add the tests before the end of the week!

@gaearon
Copy link
Contributor

gaearon commented Oct 1, 2015

Sure, thanks! Sorry I'm a bit stuck here too—on a vacation.

@pabloalonsos pabloalonsos force-pushed the feature-import-export branch from 939eb78 to 730d54e Compare October 6, 2015 18:31
@pabloalonsos
Copy link
Author

Tests added! Let me know if anything is missing

@pabloalonsos pabloalonsos force-pushed the feature-import-export branch from 730d54e to b17edef Compare October 6, 2015 21:03
@pabloalonsos pabloalonsos force-pushed the feature-import-export branch from b17edef to 4e3b362 Compare October 6, 2015 21:14
gaearon added a commit that referenced this pull request Oct 16, 2015
@gaearon
Copy link
Contributor

gaearon commented Oct 16, 2015

I'm bringing this in via b4da7aa in vNext branch.
Closing this PR as is because it won't merge cleanly there.
Thanks!

@gaearon gaearon closed this Oct 16, 2015
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