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

How to make riot.route optional? #1485

Closed
cognitom opened this issue Dec 27, 2015 · 9 comments
Closed

How to make riot.route optional? #1485

cognitom opened this issue Dec 27, 2015 · 9 comments
Labels
Milestone

Comments

@cognitom
Copy link
Member

@cognitom cognitom commented Dec 27, 2015

We've decided to make riot.route optional. #1322
I'm guessing what's the best way...

A

It could be like this:

  • riot.js
  • riot+route.js
  • riot+compiler.js
  • riot+compiler+route.js

B

Another option is loading separately:

<script src="https://cdn.jsdelivr.net/riot/2.4/riot+compiler.min.js"></script>
<script src="https://cdn.jsdelivr.net/riot/2.4/route.min.js"></script>

But it feels bit inconsistent.

B'

Should we separate compiler, too?

<script src="https://cdn.jsdelivr.net/riot/2.4/riot.min.js"></script>
<script src="https://cdn.jsdelivr.net/riot/2.4/compiler.min.js"></script>
<script src="https://cdn.jsdelivr.net/riot/2.4/route.min.js"></script>

Any thoughts? Thanks!

@cognitom
Copy link
Member Author

@cognitom cognitom commented Dec 27, 2015

BTW, for Browserify and WebPack, it'll be more simpler:

var riot = require('riot')
riot.route = require('riot-route')

or just:

const
  riot  = require('riot')
  route = require('riot-route')
@aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Dec 27, 2015

@cognitom , please B. I just was thinking about the parsers.

I think we must remove all the parsers and release riot with a subfolder with these and other optional "core" components, like the router, the compiler, maybe an alternate styleManager, and so on (custom tmpl for precompiled expressions).

Example: Include a pre-made default version of riot for browsers only --i.e. unminified, no compiler/commonjs/router.
Then, with an included tool the user can make a custom version of riot, like underscore or zepto already does:

mkriot --compiler --router --parser babel -o js/riot

just an idea.

@aMarCruz
Copy link
Contributor

@aMarCruz aMarCruz commented Dec 27, 2015

I already do something like this with the compiler using jspreproc. This allows me to generate optimized versions for the CLI and riot with the same code base.

@phillip-haydon
Copy link

@phillip-haydon phillip-haydon commented Dec 28, 2015

Optional route! YAY.

@jpodwys
Copy link

@jpodwys jpodwys commented Jan 7, 2016

I use browserify so I don't care how you separate them, but I'm excited for this!

@cognitom
Copy link
Member Author

@cognitom cognitom commented Jan 7, 2016

Thanks for your feedbacks, guys!
@aMarCruz ok, let's go with B.

TODO:

  • make a copy of this file into the root directory of riot/riot
  • remove the tests for riot/route from riot/riot
  • update the documents
@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Jan 25, 2016

I think this topic is really important because it will affect the rollup build, I will do some tests guys in the rollup branch and I will let you know the simplest solution

@TehShrike
Copy link

@TehShrike TehShrike commented Mar 14, 2016

B' appeals to me - I don't know of any advantage for them being all in the same file.

But like jpodwys, I use Browserify, so all I really care about is you pulling the trigger on some method and publishing a smaller module to npm!

@GianlucaGuarini GianlucaGuarini mentioned this issue Mar 19, 2016
14 of 16 tasks complete
@kazzkiq
Copy link
Contributor

@kazzkiq kazzkiq commented Mar 21, 2016

Agree with @TehShrike as B' seems more appealing. This helps in separating scopes and making Riot more flexible and modular.

And everyone uses task-runners and/or module-loaders nowadays anyways, all those independent files could easily be concatenated or loaded on demand in production. No big deal on performance topic.

@GianlucaGuarini GianlucaGuarini added this to the 3.0.0 milestone Oct 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.