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

fix(router-view): register instance in init hook (fix #2561) #2689

Closed
wants to merge 2 commits into from

Conversation

zrh122
Copy link
Contributor

@zrh122 zrh122 commented Mar 30, 2019

close #2561

@posva
Copy link
Member

posva commented Mar 30, 2019

Hey could you add a test for this?

An e2e test should do it

@zrh122
Copy link
Contributor Author

zrh122 commented Mar 30, 2019

Yes, i will.

@zrh122
Copy link
Contributor Author

zrh122 commented Mar 30, 2019

@posva , e2e test added.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

Thanks, I'm merging this locally after applying some modifications (splitting the e2e test to keep the navigation guard one in an existing e2e test and the keep alive in another

posva added a commit that referenced this pull request Apr 15, 2019
Fix #2561
Close ##2689

Squashed commit of the following:

commit e18dd2f660b2154b26e6afa9d60ac76b047b8e40
Author: Eduardo San Martin Morote <posva13@gmail.com>
Date:   Mon Apr 15 12:54:55 2019 +0200

    refactor: use keepalive example instead

commit 2c458af27f5371c690c2832d59e0b670e43a17fa
Author: Eduardo San Martin Morote <posva13@gmail.com>
Date:   Mon Apr 15 11:48:24 2019 +0200

    refactor: remove unused classes

commit 8467722640ee11baecabf52dd91bbc7d90d370a0
Author: zrh122 <1229550935@qq.com>
Date:   Sun Mar 31 05:26:05 2019 +0800

    test: add e2e test for #2561

commit af0ff980b4ca0b32637aa34d692587a790790947
Author: zrh122 <1229550935@qq.com>
Date:   Sat Mar 30 13:57:38 2019 +0800

    fix(router-view): register instance in init hook

    close #2561
@posva
Copy link
Member

posva commented Apr 15, 2019

Merged locally!

@posva posva closed this Apr 15, 2019
@zrh122
Copy link
Contributor Author

zrh122 commented Apr 15, 2019

@posva the e2e test for #2561 may be not correct. I removed the code (register instance in init hook), and executed npm run test:e2e, all the test case was passed. It should not be passed as i expect.

mine:

// routes
[
  // foo1
  { path: '/foo1', component: Foo },

  // foo2, same component as foo1
  { path: '/foo2', component: Foo },

  // bar
  { path: '/bar', component: Bar }
]

click order: /foo1 -> /bar -> /foo2

yours:

// routes
[  
  {
    path: '/with-guard1',
    name: 'with-guard1',
    component: WithGuard
  },
  {
    path: '/with-guard2',
    name: 'with-guard2',
    component: WithGuard
  }
]

click order: /with-guard1 -> /with-guard2 -> /with-guard1

maybe you can add another route, and the click order can be:

/with-guard1 -> /another -> /with-guard2

@posva
Copy link
Member

posva commented Apr 15, 2019

Right! Thanks for checking, I'm going to fix the test

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.

beforeRouteEnter hook's cb is not called (same component across diffrent routes and keep-alive applied )
2 participants