Better imports for SSR #1998

Closed
prateekbh opened this Issue Sep 27, 2016 · 5 comments

Projects

None yet

2 participants

@prateekbh
Contributor
prateekbh commented Sep 27, 2016 edited

Importing for Server side rendering has to be done at root level :(

  1. Describe your issue:
    Today for server side rendering we do require tag files individually.
    BUT if the tag itself require some subsequent tags and try to import/require them at the top,
    riot's require definition start throwing error:
/Users/prateekbh/projects/riot-tagrouter/tags/app-router.tag:1
(function (exports, require, module, __filename, __dirname) { var riot = require(process.env.RIOT || "riot/riot.js");module.exports =var routerTag = require("./core/router.tag");

This needs to go away and allow devs to do more friendly imports for large Isomorphic apps

  1. Can you reproduce the issue?
    1.) app-router.tag
var routerTag = require("./core/router.tag");
var routeTag = require("./core/route.tag");
var navigateTag = require("./core/navigate.tag");
var homeTag = require("./rtr-home.tag");
<app-router>
    <router>
        <route path ='/' component="rtr-home"/>
    </router>
</app-router>

2.) app.js
var riot = require("riot");
var routerTag = require('../tags/app-router.tag');
console.log(riot.render('app-router',{location:'/'}));

3.) in shell run node ./app.js

  1. On which browser/OS does the issue appear?
    node
  2. Which version of Riot does it affect?
    All future (AFAIK)
  3. How would you tag this issue?
    Feature Request
    • Question
    • Bug
    • Discussion
    • Feature request
    • Tip
    • Enhancement
    • Performance
@GianlucaGuarini
Member

Have you also tried with riot@next?

@prateekbh
Contributor

@GianlucaGuarini lemme chk, cuz I did not found any indication that this was going to be fixed

@prateekbh
Contributor
prateekbh commented Sep 27, 2016 edited

@GianlucaGuarini just checked next's code

var src = compiler.compile(fs.readFileSync(filename, 'utf8'), opts)
  var hasGlobalRiot = typeof global.riot !== 'undefined'
  if (!hasGlobalRiot) global.riot = riot
  module._compile(`module.exports = ${ src }`, filename)
  if (!hasGlobalRiot) delete global.riot

This still tries to module.exports = ${ src }, which will incur the following code and break

var routerTag = require("./core/router.tag");
.
.
.

@prateekbh
Contributor

@GianlucaGuarini submitting a PR to dev, but i see that code in next has changed, aren't they supposed to be in sync?

@prateekbh prateekbh added a commit to prateekbh/riot that referenced this issue Sep 27, 2016
@prateekbh prateekbh closes #1998 dd86621
@prateekbh
Contributor

sry, earlier pull request had some unwanted commits coming in.
Latest one(#2000) has all the required code

@GianlucaGuarini GianlucaGuarini added a commit that referenced this issue Oct 8, 2016
@GianlucaGuarini GianlucaGuarini Merge branch 'bug/1998' into next
* bug/1998:
  closes #1998 and normalize the riot.require function
  wip
3104cb3
@GianlucaGuarini GianlucaGuarini pushed a commit that closed this issue Oct 8, 2016
@prateekbh prateekbh closes #1998 b578168
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment