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

feat(utils): devtools dispatch support #260 #261

Merged
merged 11 commits into from
Nov 9, 2021

Conversation

Aslemammad
Copy link
Member

Here's a simple impl of the feature! Any suggestions/reviews are welcome.

Reminder: we discussed eval/new Function for simple js values instead of JSON values, but it's risky for now. So it's good to go with JSON for now

cc @AshConnolly

src/utils/devtools.ts Outdated Show resolved Hide resolved
src/utils/devtools.ts Outdated Show resolved Hide resolved
src/vanilla.ts Outdated Show resolved Hide resolved
@dai-shi dai-shi linked an issue Oct 23, 2021 that may be closed by this pull request
Aslemammad and others added 2 commits October 23, 2021 14:13
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
src/utils/devtools.ts Outdated Show resolved Hide resolved
src/utils/devtools.ts Outdated Show resolved Hide resolved
src/utils/devtools.ts Outdated Show resolved Hide resolved
src/utils/devtools.ts Outdated Show resolved Hide resolved
Aslemammad and others added 3 commits October 23, 2021 14:30
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
Copy link
Member

@dai-shi dai-shi left a comment

Choose a reason for hiding this comment

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

Looks great.

@AshConnolly
Copy link

Awesome! Thanks guys! Appreciate it! 😃 👍

@dai-shi
Copy link
Member

dai-shi commented Oct 23, 2021

Oh, codesandbox bot is not working.

src/utils/devtools.ts Outdated Show resolved Hide resolved
Aslemammad and others added 3 commits October 23, 2021 15:35
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2dcef56:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration

@dai-shi
Copy link
Member

dai-shi commented Oct 24, 2021

@AshConnolly Please try the codesandbox ci build, and make sure if this fixes what you want. You can install the build locally.

@AshConnolly
Copy link

AshConnolly commented Oct 28, 2021

Thanks so much for this @dai-shi & @Aslemammad! Appreciate it! Sorry for taking so long to get back to this!

I just tested it on Codesandbox and couldn't get it to work. What shape should the dispatch object be?
I tried various dispatch objects like :

{
  type: "ACTION",
  text: "bye"
}
{
  type: "ACTION",
  state: {
    text: "bye"
  }
}

but got this error:
index.js:27 please dispatch a serializable value that JSON.parse() and proxy() support SyntaxError: Unexpected token t in JSON at position 1

Sandbox here - https://codesandbox.io/s/react-forked-rhp1s?file=/src/App.js

In terms of the PR, you made this change look simple, I might even try and contribute next time I want a feature! (I'm sure its not as simple as it looks though!!)

Thanks again! 😄

@dai-shi
Copy link
Member

dai-shi commented Oct 28, 2021

I never tried, but it's probably this:

{"text":"bye"}

@AshConnolly
Copy link

So sorry for the late reply!
That appears to be working for me! 😄 🚀
see here - https://codesandbox.io/s/react-forked-euoqf?file=/src/App.js

Thanks so much! 👍

This change solves the issue of changing Valtio state from a dev tool which is great. However, I have a kind of related question that I hinted at here...

Is it possible to have some sort of Valtio implementation that allows us to change Valtio state from react dev tools? I often wonder if people are hesitant to use state libraries that don't clearly show state in devtools. Like with context you can see and modify context state in the react devtools, which is nice.

Happy to raise this as a separate issue / discussion if it helps!

Thanks again! 😄

@dai-shi
Copy link
Member

dai-shi commented Nov 9, 2021

Feel free to open a new issue / discussion. Maybe discussion, as we don't know if it's possible yet?

@dai-shi dai-shi merged commit f7f4235 into pmndrs:main Nov 9, 2021
@dai-shi dai-shi changed the title feat: devtools dispatch support #260 feat(utils): devtools dispatch support #260 Nov 9, 2021
@AshConnolly
Copy link

Fantastic! Thanks so much @dai-shi & @Aslemammad! 😃 👍

@Noitidart
Copy link
Contributor

Noitidart commented Nov 10, 2021

Thanks @AshConnolly for bringing ideas from other places to here. I really think Valtio has a bright future! Taking the best ideas from other state management solutions and bringing it here.

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.

Is it possible to update Valtio state from redux devtools?
4 participants