Skip to content

Commit

Permalink
Simplify device type routing (#950)
Browse files Browse the repository at this point in the history
* Simplify device type routing

The mobile-detect package is large. We shouldn't send it to the browser.

This change uses mobile-detect on the server only. It associates a device
type with the request and sends only that pre-determined value to the browser.

This eliminates the "mobile" route target, which was previously the union of
"phone" and "tablet". These two must be specified individually going forward
(though they may route to the same page class). It also adds a "desktop" route
target.

The `getMobileDetect()` method on the request context is eliminated, and
replaced with a `getDeviceType()` method that returns a string.

This is a breaking change.

* Eliminate a now-unused variable (and import)

* Restore route name device type suffix

* Actually remove mobile
  • Loading branch information
gigabo authored Sep 12, 2017
1 parent f8c483c commit 2f63705
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 44 deletions.
4 changes: 1 addition & 3 deletions packages/react-server/core/ClientController.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

var React = require('react'),
ReactDOM = require('react-dom'),
MobileDetect = require('mobile-detect'),
logging = require('./logging'),
RequestContext = require('./context/RequestContext'),
RequestLocalStorage = require('./util/RequestLocalStorage'),
Expand Down Expand Up @@ -71,6 +70,7 @@ class ClientController extends EventEmitter {
}

this.context = buildContext(routes);
this.context.setDeviceType(dehydratedState.deviceType);
ReactServerAgent.cache().rehydrate(dehydratedState.InitialContext['ReactServerAgent.cache']);
this.mountNode = document.getElementById('content');

Expand Down Expand Up @@ -844,8 +844,6 @@ function buildContext(routes) {
.setRoutes(routes)
.create();

context.setMobileDetect(new MobileDetect(navigator.userAgent));

return context;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class RequestContextStub {
this.navigator.navigate(request, type);
});
}
getMobileDetect() { return null; }
getDeviceType() { return null; }
}


Expand Down
47 changes: 12 additions & 35 deletions packages/react-server/core/context/Navigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ var EventEmitter = require('events').EventEmitter,
DebugUtil = require("../util/DebugUtil"),
{setResponseLoggerPage} = SERVER_SIDE ? require('../logging/response') : { setResponseLoggerPage: () => {} };

var _ = {
isFunction: require('lodash/isFunction'),
};

class Navigator extends EventEmitter {

constructor (context, routes) {
Expand Down Expand Up @@ -83,45 +79,26 @@ class Navigator extends EventEmitter {

var loaders = route.config.page;

// Normalize.
// The page object may either directly be a loader or
// it may be an object whose values are loaders.
if (_.isFunction(loaders)){
loaders = {'default': loaders};
}
var deviceType = this.context.getDeviceType();

var mobileDetect = this.context.getMobileDetect();
if (loaders[deviceType]) {
route.name += "-" + deviceType;
}

// Our route may have multiple page implementations if
// there are device-specific variations.
//
// We'll take one of those if the request device
// matches, otherwise we'll use the default.
//
// Note that 'mobile' is the _union_ of 'phone' and
// 'tablet'. If you _really_ want an iPad and an
// iPhone to get the _same_ non-desktop experience,
// use that.
//
var loadPage = [
'phone',
'tablet',
'mobile',
].reduce((loader, format) => {

// We'll take the _first_ format that matches.
if (!loader && loaders[format] && mobileDetect[format]()){

// Need to disambiguate for bundleNameUtil.
route.name += '-'+format;

return loaders[format];
}

return loader;
}, null) || loaders.default;

loadPage().done(pageConstructor => {
// Note that the page object may either directly be a
// loader or it may be an object whose values are
// loaders.
(
loaders[deviceType] ||
loaders.default ||
loaders
)().done(pageConstructor => {
if (request.setRoute) {
request.setRoute(route);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/react-server/core/context/RequestContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ class RequestContext {
return this.serverStash;
}

setMobileDetect (mobileDetect) {
this.mobileDetect = mobileDetect;
setDeviceType (type) {
this.deviceType = type;
return this;
}

getMobileDetect () {
return this.mobileDetect;
getDeviceType () {
return this.deviceType;
}

getCurrentPath () {
Expand Down
14 changes: 13 additions & 1 deletion packages/react-server/core/renderMiddleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ module.exports = function(req, res, next, routes) {
// Need this stuff in for logging.
context.setServerStash({ req, res, start, startHR });

context.setMobileDetect(new MobileDetect(req.get('user-agent')));
context.setDeviceType(getDeviceType(req));

var navigateDfd = Q.defer();

Expand Down Expand Up @@ -945,6 +945,7 @@ function bootstrapClient(res, lastElementSent) {

var initialContext = {
'ReactServerAgent.cache': ReactServerAgent.cache().dehydrate(),
'deviceType': RequestContext.getCurrentRequestContext().getDeviceType(),
};

res.expose(initialContext, 'InitialContext');
Expand Down Expand Up @@ -1070,6 +1071,17 @@ function getNonInternalConfigs() {
return nonInternal;
}

function getDeviceType(req) {
var md = new MobileDetect(req.get('user-agent'));
var types = [ 'phone', 'tablet' ];
for (var i = 0; i < types.length; i++) {
if (md[types[i]]()) {
return types[i];
}
}
return 'desktop';
}

module.exports._testFunctions = {
renderMetaTags,
renderLinkTags,
Expand Down

0 comments on commit 2f63705

Please sign in to comment.