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.

Sign up for GitHub

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

Jump to bottom

feat(devtools): useAtomsDevtools #837

Merged
merged 63 commits into from
Dec 30, 2021
Merged

feat(devtools): useAtomsDevtools #837

merged 63 commits into from
Dec 30, 2021

Conversation

Aslemammad
Copy link
Member

@Aslemammad Aslemammad commented Nov 13, 2021 •
edited by dai-shi

useAtomsDevtools #811

  • initial trial
  • pass tests
  • batch actions
  • avoid duplicated logs
  • make dependencies work (they work in the tests)
  • overall behavior checks
  • make dependencies viewable
  • investigate to avoid set timeout
  • investigate to avoid create microtask
  • fix time-travel feature (JUMP_TO_STATE)
  • add test for JUMP_TO_STATE
  • add docs
  • refactor code
  • test with <StrictMode>
  • instead of dependencies, use dependents like <Provider>
  • create a demo with many atoms with changing dependencies
  • fix failing tests (re-implement)
  • fix JUMP_TO_STATE in codesandbox 👉 a slight modification works with <AtomsDevtools>
  • fix failing tests (unstable_versionedWrite)
  • modify docs with <AtomsDevtools> practice

@vercel
Copy link

vercel bot commented Nov 13, 2021 •
edited

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/jotai/748co3wGbXksPzxZW5K1zRDMwfQh
✅ Preview: https://jotai-git-fork-aslemammad-feat-useatomsdevtools-pmndrs.vercel.app

@Aslemammad Aslemammad changed the title feat: init feat: useAtomsDevtools Nov 13, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 13, 2021 •
edited

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 d8ec620:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
Next.js Configuration
Next.js with custom Babel config Configuration
React with custom Babel config Configuration
React Typescript (forked) PR
React Typescript (forked) PR

@dai-shi dai-shi marked this pull request as draft November 13, 2021 12:03
@Aslemammad @dai-shi
change error text
43d7636 
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
dai-shi
dai-shi reviewed Dec 13, 2021
View reviewed changes
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.

Please fix lint errors and tests too.

tests/devtools/useAtomsDevtools.test.tsx Outdated
describe('when it receives an message of type...', () => {
describe('DISPATCH and payload of type...', () => {
describe('JUMP_TO_STATE | JUMP_TO_ACTION...', () => {
it('time travelling', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a little bit more complex scenario?

Copy link
Member

Choose a reason for hiding this comment

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

@Aslemammad Do you have any other ideas to add some more tests?

src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated
Comment on lines 119 to 120
case 'JUMP_TO_STATE':
case 'JUMP_TO_ACTION':
Copy link
Member

Choose a reason for hiding this comment

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

should we only support JUMP_TO_ACTION? JUMP_TO_STATE doesn't make sense, and we don't have actionId, do we?

src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
src/devtools/useAtomsDevtools.ts Outdated Show resolved Hide resolved
@dai-shi dai-shi mentioned this pull request Dec 30, 2021
Merged
dai-shi
dai-shi approved these changes Dec 30, 2021
View reviewed changes
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.

finally, it looks good.

@Aslemammad
Copy link
Member Author

Finally!

@dai-shi dai-shi merged commit ee9e8b3 into pmndrs:main Dec 30, 2021
@Aslemammad Aslemammad deleted the feat/useAtomsDevtools branch December 30, 2021 11:46
@dai-shi dai-shi changed the title feat: useAtomsDevtools feat(devtools): useAtomsDevtools Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dai-shi dai-shi dai-shi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.5.0
Development

Successfully merging this pull request may close these issues.

devtools: useAtomsDevtools
2 participants
@Aslemammad @dai-shi