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

Projects
None yet
2 participants
@zrh122
Copy link
Contributor

zrh122 commented Mar 30, 2019

close #2561

@posva

This comment has been minimized.

Copy link
Member

posva commented Mar 30, 2019

Hey could you add a test for this?

An e2e test should do it

@zrh122

This comment has been minimized.

Copy link
Contributor Author

zrh122 commented Mar 30, 2019

Yes, i will.

@zrh122

This comment has been minimized.

Copy link
Contributor Author

zrh122 commented Mar 30, 2019

@posva , e2e test added.

@zrh122 zrh122 force-pushed the zrh122:fix/2561 branch from c480f55 to 4a3a51a Apr 3, 2019

@posva
Copy link
Member

posva left a comment

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(router-view): register instance in init hook
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

This comment has been minimized.

Copy link
Member

posva commented Apr 15, 2019

Merged locally!

@posva posva closed this Apr 15, 2019

@zrh122

This comment has been minimized.

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

This comment has been minimized.

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
You can’t perform that action at this time.