Skip to content

Server-side support#19

Merged
GianlucaGuarini merged 1 commit intoriot:devfrom
gpoitch:ss
Nov 9, 2015
Merged

Server-side support#19
GianlucaGuarini merged 1 commit intoriot:devfrom
gpoitch:ss

Conversation

@gpoitch
Copy link
Contributor

@gpoitch gpoitch commented Sep 21, 2015

  • All code can now be safely executed on the server
  • The exec method now takes an optional path parameter which can be used on the server. This allows the server to use your route handling logic to control what's rendered.

@GianlucaGuarini
Copy link
Member

cool! @gdub22 thanks!
@cognitom I will let you merge and QA the pull requests on the riot-route repo is it ok?

@cognitom
Copy link
Member

@GianlucaGuarini sure. Will do.

@gdub22 Thanks! I'm bit busy now, I'll check it tomorrow ;)

@cognitom
Copy link
Member

@gdub22 I'm a bit new to the server-side Riot, so it's been taking a time to review it. Thank you for waiting.
How about separating the test code from front-end testing? The event handling on browser seems not needed in server-side.

@gpoitch
Copy link
Contributor Author

gpoitch commented Sep 27, 2015

Thanks for reviewing. I can separate the tests when I get some time. I kept it in the same file because it was easier to run the same specs on both browser and server.

@cognitom
Copy link
Member

cognitom commented Nov 1, 2015

@gdub22 I'm going to merge this after releasing of v2.3.0 of Riot.js. I'll be back soon. Sorry.

@gpoitch
Copy link
Contributor Author

gpoitch commented Nov 1, 2015

Excellent! I just fixed the conflicts

GianlucaGuarini added a commit that referenced this pull request Nov 9, 2015
@GianlucaGuarini GianlucaGuarini merged commit c95114a into riot:dev Nov 9, 2015
@GianlucaGuarini
Copy link
Member

@cognitom I am having problems merging the dev branch into the master. Can you please fix the conflicts and make a new release 2.3.1? So we can bump a new riot patch release

@cognitom
Copy link
Member

cognitom commented Nov 9, 2015

@GianlucaGuarini can I roll back this merge commit? I need to check this PR more, because some API has been changed. It seems better to focus to only fix for v2.3.1.

@GianlucaGuarini
Copy link
Member

@cognitom ok sorry I thought It wasn't that hard

@cognitom
Copy link
Member

cognitom commented Nov 9, 2015

No prob ;-)

@cognitom cognitom mentioned this pull request Nov 14, 2015
2 tasks
@gpoitch
Copy link
Contributor Author

gpoitch commented Dec 15, 2015

@cognitom @GianlucaGuarini Besides resolving any conflicts, what can I do to get this re-merged?

@GianlucaGuarini
Copy link
Member

I would like to give the lead to @cognitom on this because last time I merged It was a disaster.. 😄

@cognitom
Copy link
Member

Oh, sorry. What I have to do is just making a time. I'm still not familiar with server-side riot.

@GianlucaGuarini
Copy link
Member

I am not even sure it's needed.. I ended up with express and this solution that works pretty well https://github.com/GianlucaGuarini/riot-app-example/blob/master/shared/routes.js

@gpoitch
Copy link
Contributor Author

gpoitch commented Dec 16, 2015

@cognitom @GianlucaGuarini If this will help you, I can break it up into a few PRs:

  1. Just adding guards so the library doesn't crash on the server.
  2. Add or change any APIs so it can be used on the server.
  3. Add separate tests with a functioning http server.

FYI I'm already using this successfully in production.

@GianlucaGuarini
Copy link
Member

@gdub22 can you please update your pull request making a new one on the current codebase?

@gpoitch
Copy link
Contributor Author

gpoitch commented Dec 19, 2015

@GianlucaGuarini yes, could you sync the dev branch? It looks far behind master

@GianlucaGuarini
Copy link
Member

I guess @cognitom could help you on that I don't want to destroy everything again like I did last time. Could you collaborate you guys working on this task?

@GianlucaGuarini
Copy link
Member

@gdub22 btw I have merged the master into the dev branch so you can start patching it

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.

3 participants