Skip to content
This repository has been archived by the owner on May 8, 2021. It is now read-only.

Permutations support required #11

Open
jherr opened this issue Jan 22, 2015 · 7 comments
Open

Permutations support required #11

jherr opened this issue Jan 22, 2015 · 7 comments
Milestone

Comments

@jherr
Copy link

jherr commented Jan 22, 2015

Hula-hoop has an issue with the multiple permutations on Zeus projects where hybrid is set to true.

/Users/jherri1/master/pharmacy/node_modules/hapi/node_modules/hoek/lib/index.js:663
    throw new Error(msgs.join(' ') || 'Unknown error');
          ^
Error: Invalid route options ([object Object]) {
  "method": "GET",
  "path" [1]: {
    "children": {
      "$hybrid:android;": "android",
      "$hybrid:ios;": "ios",
      "$hybrid:browser;": "browser"
    }
  }
}

[1] path must be a string
    at Object.exports.assert (/Users/jherri1/master/pharmacy/node_modules/hapi/node_modules/hoek/lib/index.js:663:11)
    at Object.exports.assert (/Users/jherri1/master/pharmacy/node_modules/hapi/lib/schema.js:15:10)
    at new module.exports.internals.Route (/Users/jherri1/master/pharmacy/node_modules/hapi/lib/route.js:39:12)
    at internals.Connection._addRoute (/Users/jherri1/master/pharmacy/node_modules/hapi/lib/connection.js:344:17)
    at internals.Connection._route (/Users/jherri1/master/pharmacy/node_modules/hapi/lib/connection.js:336:18)
    at internals.Plugin._apply (/Users/jherri1/master/pharmacy/node_modules/hapi/lib/plugin.js:432:14)
    at internals.Plugin.route (/Users/jherri1/master/pharmacy/node_modules/hapi/lib/plugin.js:407:10)
    at Object.exports.register (/Users/jherri1/master/pharmacy/src/server/index.js:13:10)
    at /Users/jherri1/master/pharmacy/node_modules/hapi/lib/plugin.js:235:14
    at iterate (/Users/jherri1/master/pharmacy/node_modules/hapi/node_modules/items/lib/index.js:35:13)

Where the code at server/index.js line #13 is:

  server.route(
    HulaHoop.api.resourceLoader.routes().map(function(route) {
      return {
        path: route,
        method: 'GET',
        handler: HulaHoop.endpoints.page(
          appName,
          {resourceRoot: '/r'}
        )
      };
    })
  );
@nvcexploder
Copy link
Contributor

howdy @jherr. I'm starting to take a look at this now.

@nvcexploder
Copy link
Contributor

@jherr I can't find "children ..." in the pharmacy project. I see it in the pack project in node_modules. What behavior is causing this error?

@kpdecker
Copy link
Contributor

kpdecker commented Feb 6, 2015

@nvcexploder the issue is the structure that is created when permutations are in play results in a circus.json at the root of the build dir that only references the children, which have the actual data needed by hula-hoop to load.

The issues are:

  1. Making hula-hoop aware of the children linking when built this way.
  2. How do we differentiate between the urls in these cases? We need some friendly, and configurable, way of differentiating between the routes. This could be path prefix or it could be via a query parameter, but we need to make sure that:
    a. All routes can still be loaded
    b. The browser urls don't have unnecessary cruft in their urls. I.e. if we use a prefix, /ios/foo and /android/foo are fine but we don't want /browser/foo, instead /foo.
  3. Make sure that the server permutations are used in favor of the client ones, when available.

This might require injecting additional data into the circus.json files to make sure that we get all of the metadata that we need and if so, I can point you to examples of how to do this.

@nvcexploder
Copy link
Contributor

Initial takes on this make 'browser' look like a keyword, should we implement that way or should we go with a 'default', or both?

@kpdecker
Copy link
Contributor

kpdecker commented Feb 9, 2015

@nvcexploder I'd like to avoid parsing the configId, if possible. I think we can use the serverRender flag that is generated by our internal tooling to help with some of this, if need be. We can also add additional metadata to circus.json, if need be.

@kpdecker kpdecker modified the milestone: 1.1.0 Feb 13, 2015
@kpdecker
Copy link
Contributor

Taking a look at what needs to happen here. Basically we have to dedupe and provide namespaces for each permutation. The general rules are as follows:

  • The glue project are expected to set their prefix value. I.e. Pack internally
  • For a given prefix, multiple routes may exist only if they have different serverRender flags
  • For a given prefix + route, the serverRender flag will take precedence if set.

This allows for conditionals like:

Zeus.router();  // Run in client and server
if (!$serverSide) {
  Zeus.router(); // Only run in client, but server will send CJS response
}

Should the need for such behavior arise. If this is not atomic enough, then we can look at other attribute setups later.

@jherr
Copy link
Author

jherr commented Mar 28, 2015

So what is the 'glue' project in this architecture? Would that be the application project (e.g. pharmacy)? And I'm ok with server rendering not being part of an initial pass here if that helps.

Also, Ken Wheeler is working on our hybrid stuff now. He can certainly happily zing along without this support. But eventually, say a month from now, the need will be a little more urgent. So, thanks for looking into this now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants