riot / route

Simple isomorphic router
MIT License
212 stars 40 forks source link

riot-route-tag: tag based router for Riot.js #79

Closed cognitom closed 4 years ago

cognitom commented 7 years ago

I'm thinking about tag-based router as a new feature. I'm now working on feature/route-tag branch and I'd like to share my current idea here. Nothing was decided yet. Any feedback is welcome!

Todos

Specs (proposal)

<app>
  <router>
    <route path="fruit/apple"><p>Apple</p></route>
    <route path="fruit/banana"><p>Banana</p></route>
    <route path="fruit/*"><inner-tag /></route>
  </router>
</app>

<inner-tag>
  <p>{ name } is not found</p>
  <script>
    this.on('route', name => this.name = name)
  </script>
</inner-tag>

Two builds:

We could import them like this:

import route from 'riot-route'
// or
import route from 'riot-route/lib/tag' // note that it's not same as cjs below
const route = require('riot-route')
// or
const route = require('riot-route/tag')

Q&A

Related informations:

GianlucaGuarini commented 7 years ago

I like the idea of having require('riot-route/tag') but I am not sure what it should return though. Another thing to consider is the new riot-hot-reload feature that will land soon in riot https://github.com/riot/riot/pull/2095, how will these new features work together?

cognitom commented 7 years ago

@GianlucaGuarini I like the idea require('riot-route/tag'), too. But I don't know any good way to provide the code for ES6. jsnext:main or module in package.json is the way to do that but only for the main module of the package.

Alternatively we could use .mjs extension for ES6, but it seems fading away from the proposal...?

BTW, the discussion below seems changing day by day. So I've started thinking about the option to have two npm packages.

I'm not sure about hot reloading, but it should work. The tags are basically just normal Riot tags πŸ˜„

GianlucaGuarini commented 7 years ago

@cognitom I would just compile the riot tags in a bundle and provide them under riot-route/tag. You can compile them using riot -m and you will get UMD wrappers ready for the dist version. I wouldn't even think aboutmain:next

cognitom commented 7 years ago

@GianlucaGuarini UMD doesn't support ES6 modules...

Could you check ES6 and CJS? It could not be an option to provide the naked tags for ES6/CJS (rollup/webpack/browserify) without compilation, either. Because there's no interoperability between them at this point.

GianlucaGuarini commented 7 years ago

I thought rollup could handle also handle CJS https://github.com/rollup/rollup-plugin-commonjs.

GianlucaGuarini commented 7 years ago

this file also does not really make sense. In case our users don't use a bundler we can assume that route (or riot.route) and riot are globally available

cognitom commented 7 years ago

@GianlucaGuarini thanks for your feedback! I thought this:

<script src="https://cdn.jsdelivr.net/riot/3.0.5/riot.js"></script>
<script src="https://cdn.jsdelivr.net/riot-route/3.1.0/route-tag.js"></script>

would be better than this:

<script src="https://cdn.jsdelivr.net/riot/3.0.5/riot.js"></script>
<script src="https://cdn.jsdelivr.net/riot-route/3.1.0/route.js"></script>
<script src="https://cdn.jsdelivr.net/riot-route/3.1.0/route-tag.js"></script>

There's no use case we use route-tag.js separately in the latter case. (remember that we're not hosting compiler.js in riot/riot any more)

How about route+tag.js instead of route-tag.js?

cognitom commented 7 years ago

Yeah, Rollup could handle CJS but I just don't want to... :sob: Anyone would use CJS in frontend projects in 2017?

GianlucaGuarini commented 7 years ago

@cognitom You know what? At this point I would prefer a separate repo for the route-tag and I would call it something like riot-pequod or riot-nautilus to make its name a bit more appealing and less sad than route-tag. In that repo you could simply add the riot-route and riot as dependencies. I guess it does not make sense for other users using only riot-route without riot, to have the riot dependency just because a sub-package of riot-route uses custom tags.

cognitom commented 7 years ago

@GianlucaGuarini that's exactly why I've separated it into two. Ok, but I don't want to push it more. I was just looking for a simpler solution :expressionless: Let's go with one package with two js files in the root dir:

and existing these files:

Looks ok?

Updated: es.tag.js --> lib/tag.js for the future compatibility

cognitom commented 7 years ago

@GianlucaGuarini thanks!

I've rebranched as feature/route-tag and cleaned it up πŸ˜„

cognitom commented 7 years ago

Related to this issue, I've sent https://github.com/rollup/rollup-plugin-node-resolve/pull/69. If it has been accepted, we'll be able to write simply like this:

import route from 'riot-route/tag'
cognitom commented 7 years ago

OK, v3.1.0 has been released just now with our new tag based router πŸŽ‰

GianlucaGuarini commented 7 years ago

congrats good job @cognitom

karantir commented 7 years ago

Is there a plan to implement lazy loading/compiling for inner tags? Something like that:

<app>
  <router>
    <route path="fruit/*"><inner-tag /></route>
    <route path="veggie/*" tag="lazy-inner-tag"></route>
  </router>
</app>
cognitom commented 7 years ago

@karantir sounds interesting. How about simply making seperated <lazy> loading tag?

<app>
  <router>
    <route path="fruit/*"><inner-tag /></route>
    <route path="veggie/*"><lazy src="tags/lazy-inner-tag.tag" /></route>
  </router>
</app>

I think we don't have to couple the router and lazy loader, at this point.

GianlucaGuarini commented 4 years ago

Closing this issue because it's related to an old router version. Please update to the latest @riotjs/route version if you can.