Skip to content

Design for Vue 3 #3124

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

Closed
jods4 opened this issue Feb 13, 2020 · 15 comments
Closed

Design for Vue 3 #3124

jods4 opened this issue Feb 13, 2020 · 15 comments

Comments

@jods4
Copy link

jods4 commented Feb 13, 2020

What problem does this feature solve?

The roadmap for Vue 3 says it will release Q1 2020 and the core lib is already very usable.
I think now is the right time to port vue-router, which is a must-have for many apps.
The exercise may provide interesting feedback on Vue3 public APIs.

I could help porting the code but I'm unsure of some design decisions.

What does the proposed API look like?

1. Setup

install could be an instance function on VueRouter so that one could register it like that:

let router = new VueRouter(/* routes, options */);  // <- actual, usable router
createApp(MyApp)
  .use(router)
  .mount('#app');

What install does is:

  • Register global components router-link and router-view;
  • Provide value Router at the root of application.
  • ❓ Do we want to provide a Route ref as well? It's available through the router object.

I'll talk more about injecting value next.
If someone wants more control (e.g. use different name for components or not register them globally), those tasks can easily be made manually instead of using .use(router).

2. Accessing the router

There is no global mixins $router, $route anymore.

If a component (userland, or internally in router-view and router-link) needs to access the router, e.g. to navigate, it injects the router in setup: let router = inject('Router').

The route can be found on the router, or could be provided as Route (ref).
Nesting routers can be done be re-providing those keys.

3. Guards

This is what I'm having most trouble with.
Currently vue-router does some ugly hacks to get hold of component instances that are navigated to/from, in order to call beforeRouteEnter, beforeRouteUpdate and beforeRouteLeave.

I'm hoping the new API could look something like:

import { beforeRouteLeave } from 'vue-router';

export default {
  setup() {
    beforeRouteLeave(() => ...);
  }
}

but I'm not clear about how fine details would work. Such as:

  • where/how is the component defined in routing actually created?
  • is setup always called, even when using keep-alive?

With some feedback on the design and a few pointers for the last point, I can have a try at porting this component.

BTW: 300Mo of dependencies to build a 20Ko lib is crazy :(

@posva
Copy link
Member

posva commented Feb 13, 2020

Hello, it's great you want to help, we could definitely use some help! However I need you to wait a little bit more (few weeks) as there is already a lot of work already done for the next version of vue router and porting the current version would be a waste of time.
The truth is you can already experiment with it at https://www.npmjs.com/package/@posva/vue-router-next (take a look at the code in the playground folder for an example) but the source code is still going under some changes and will be public very soon.

Good news is 1 and 2 are already that way.
For 3, the api is likely to be onBeforeRoute* but we still need to figure out some changes regarding keep alive and suspense (specifically for beforeRouteEnter)

If you find things while using the package linked above, please post them here (even if the issue is closed) and ping me until the repo for vue-router-next is public and discussions can happen there with pointers at the source code as well

@posva posva closed this as completed Feb 13, 2020
@jods4
Copy link
Author

jods4 commented Feb 13, 2020

@posva cool, thanks.
I looked at issues and branches here but couldn't find anything.

I'll try it tomorrow. I'm only doing basic stuff but if I find something I'll let you know.

@jods4
Copy link
Author

jods4 commented Feb 14, 2020

@posva I'm only doing basic stuff but seems to work well enough!

Feedback:

  • Bug: package.json field types points at the wrong location dist/index.d.ts instead of dist/src/index.d.ts, which breaks the TS experience. Would be nice if you could publish a release with this fixed, I hacked the file directly.
  • API design: should history: createHistory() be a default? Seems like boilerplate.
  • API design: I'm using createHistory() (resolves to HTML 5 pushstate), so I guess the hash-based history is not tree-shakable. Not a big deal but would be a bonus.
  • Based on those 2 previous points: given that Vue 3 is aiming at modern stuff, should history: html5 be the default, hash-based is tree-shakable and used by importing either createHistory() (auto-detect) or createHistoryHash() (explicit)?

@posva
Copy link
Member

posva commented Feb 14, 2020

Thanks! I'm fixing the types and publishing a new version

For the other 3 points: the idea behind always passing history is to allow the history to be customized and also to be tree-shakeable. Because of that, I don't think there will be a default (there is an RFC about changing the default btw) but we will likely keep using the hash for beginner examples and explain why you should use the HTML5 one in real apps (see the mentioned RFC for the rationale)

@jods4
Copy link
Author

jods4 commented Feb 14, 2020

oh, I was mislead by the name createHistory(), I thought it would detect what the browser supports (html5 by default, fallback to hash otherwise).

Now I see that it's actually the html5 implementation.
Maybe rename createHtml5History for clarity? All the other ones have their implementation in their name.

@jods4
Copy link
Author

jods4 commented Feb 15, 2020

Here's another piece of feedback:
It's really nice that you thought of augmenting the inject definition to provide strongly typed router and route.

I had a funny issue with that declaration, though. I wanted to inject the router in my component and encountered this problem: microsoft/TypeScript#36812.
I don't think that you can do anything about it, except maybe give a hint in the docs.

I don't think that you have to redeclare the generic inject overloads, do you? If you did that would mean no two libraries could augment inject with different keys. ☹️ And you would need to keep them sync with the official ones.

@posva
Copy link
Member

posva commented Feb 17, 2020

Probably something like createWebHistory and createWebHashHistory. I rather not use html5 in the name because it relates to the version of html and doesn't feel right for a name

I was aware of the problem, and we will likely expose functions named useRouter and useRoute that do the injection for us. It's nice you found a solution to the problem, I'm curious about where to put that reference comment though

@jods4
Copy link
Author

jods4 commented Feb 17, 2020

I agree having a useRouter feels even better: no need to know the magic string 'router' and you get a typed Router back.

I'm curious about where to put that reference comment though

What I ended up doing is this:
I already have a types.d.ts file in my project anyway, it contains the required declare module '*.vue'.
I just added the triple-slash comment in there.

Because this .d.ts file is in my project (as opposed to provided by a node module) it's always loaded and active in every file.
So now I can do inject('router') anywhere and it works.

@mehcode
Copy link

mehcode commented Feb 17, 2020

@posva Can you export the Link and View components so we can easily consume them from JSX/TSX?

@posva
Copy link
Member

posva commented Feb 17, 2020

@mehcode yeah, this time it's possible because we only have named exports

@hareku
Copy link
Contributor

hareku commented Feb 18, 2020

@posva
I appreciate if you can release a new version that exports Link and View components.

@posva
Copy link
Member

posva commented Feb 18, 2020

@hareku it's published 🙂 Keep in mind the naming of exports is subject to change

@Destiner
Copy link

Seems like empty route ('/') doesn't work if the name is not provided.

const routerHistory = createWebHistory();
const router = createRouter({
	history: routerHistory,
	routes: [
		{ path: '/', component: Home }, // note that name is not provided
		{ path: '/user/:id', component: User },
	],
});

const app = createApp(App);
  • vue @ ^3.0.0-alpha.7
  • vue-router @ ^4.0.0-alpha.1

@posva
Copy link
Member

posva commented Feb 29, 2020

@Destiner That was already fixed but not released yet

@posva
Copy link
Member

posva commented Mar 20, 2020

There are a few more things to pickup at https://github.com/vuejs/vue-router-next/projects/2 right now. Let me know if you are working on something so I can move the task on the board

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

No branches or pull requests

5 participants