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

UPDATE for Vue3 ( = vuex 4.x, vue-router 4.x) #100

Merged
merged 12 commits into from
Feb 5, 2021

Conversation

Spice-Z
Copy link

@Spice-Z Spice-Z commented Nov 23, 2020

To use vuex-router-sync in Vue 3.x, I updated vue, vuex and vue-router!
Changed src and test, currently the test is not perfect, help...😭

About vue-routers's afterEach

  • To mutate latest route info, this package using vue-router's afterEach function.
  • But vue-router v4.x only export add function, and doesn't export any list that registered afterEach.
  • I want to test unsync correctly remove router's afterEach hook, but I have no idea.

About vuex's watch

  • To sync router on store change, this package using vuex's watch function
  • But I can't find a way to confirm what is registered in watch hooks!
  • I want to test unsync correctly remove vuex's watch hook, but I have no idea...

About unsync test

  • Old unsync tests used some functions of vuex and vue-router.
  • But in Vue 3.x, I cannnot use those functinos.
  • I set new tests, directly watch store's state.

@Spice-Z Spice-Z marked this pull request as ready for review November 23, 2020 08:00
package.json Outdated Show resolved Hide resolved
@Akryum Akryum requested review from kiaking and posva December 30, 2020 12:55
@Spice-Z Spice-Z requested a review from Akryum January 5, 2021 15:06
Comment on lines +62 to +64
"vue": "^3.0.5",
"vue-router": "^4.0.2",
"vuex": "^4.0.0-rc.2"
Copy link
Author

Choose a reason for hiding this comment

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

I want to update other dependencies, but that should be other PR to keep it simple...

src/index.ts Outdated Show resolved Hide resolved
test/index.spec.ts Outdated Show resolved Hide resolved
router,
render: (h) => h('router-view')
}).$mount()
const rootEl = document.createElement('div')
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be easier and safer to use vue test utils?

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I will try to use vue test utils!
(Actually, I have not use it yet 😅 )

@kiaking
Copy link
Member

kiaking commented Feb 4, 2021

Thank you so much for the PR! I've added test for the time traveling and now the coverage is 100% 🙌

As @posva mentioned, I think we should use vue-test-utils ideally but since it's quite small test, I would go ahead and merge this one, then maybe add playground to do the final manual test, then I think we're ready to go 👍

@kiaking kiaking changed the base branch from master to 6.x February 4, 2021 16:35
@kiaking kiaking merged commit 3dd210b into vuejs:6.x Feb 5, 2021
kiaking added a commit that referenced this pull request Feb 5, 2021
Co-authored-by: Kia King Ishii <kia.king.08@gmail.com>
kiaking added a commit that referenced this pull request Feb 5, 2021
Co-authored-by: Kia King Ishii <kia.king.08@gmail.com>
kiaking added a commit that referenced this pull request Feb 5, 2021
Co-authored-by: Kia King Ishii <kia.king.08@gmail.com>
kiaking added a commit that referenced this pull request Feb 5, 2021
Co-authored-by: Kia King Ishii <kia.king.08@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants