Add guards to decodeEntieties so it is not called with undefined. (#6551

) Add guards to decodeEntieties so it is not called with undefined. Remove decodeEntietis from SSR check Move Head to proper place. Move makeElement to place where it is used. CamelCase siteName
Automattic · Aug 1, 2016 · 9ea12f5 · 9ea12f5
1 parent c6a0d02
commit 9ea12f5
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 67 deletions.
diff --git a/client/layout/head/index.jsx b/client/layout/head/index.jsx
@@ -9,7 +9,7 @@ import debugFactory from 'debug';
 
 const debug = debugFactory( 'calypso:layout:head' );
 
-const Head = ( { title, description, canonicalUrl, type, site_name, image, children } ) => (
+const Head = ( { title, description, canonicalUrl, type, siteName, image, children } ) => (
 	<div>
 		<Helmet
 			title={ title }
@@ -18,7 +18,7 @@ const Head = ( { title, description, canonicalUrl, type, site_name, image, child
 				title ? { property: 'og:title', content: title } : {},
 				canonicalUrl ? { property: 'og:url', content: canonicalUrl } : {},
 				type ? { property: 'og:type', content: type } : {},
-				site_name ? { property: 'og:site_name', content: site_name } : {},
+				siteName ? { property: 'og:site_name', content: siteName } : {},
 				image ? { property: 'og:image', content: image } : {},
 			] }
 			link={ [
@@ -35,7 +35,12 @@ Head.propTypes = {
 	title: React.PropTypes.string,
 	description: React.PropTypes.string,
 	canonicalUrl: React.PropTypes.string,
+	type: React.PropTypes.string,
+	siteName: React.PropTypes.string,
+	image: React.PropTypes.string,
 	children: React.PropTypes.node,
 };
 
+Head.defaultProps = { site_name: 'WordPress.com' };
+
 export default Head;
diff --git a/client/my-sites/theme/controller.jsx b/client/my-sites/theme/controller.jsx
@@ -2,43 +2,25 @@
  * External Dependencies
  */
 import React from 'react';
-import omit from 'lodash/omit';
 import debugFactory from 'debug';
 import startsWith from 'lodash/startsWith';
-import i18n from 'i18n-calypso';
 
 /**
  * Internal Dependencies
  */
 import ThemeSheetComponent from './main';
 import ThemeDetailsComponent from 'components/data/theme-details';
 import { getCurrentUser } from 'state/current-user/selectors';
-import { getThemeDetails } from 'state/themes/theme-details/selectors';
 import {
 	receiveThemeDetails,
 	receiveThemeDetailsFailure,
 	setBackPath
 } from 'state/themes/actions';
 import wpcom from 'lib/wp';
 import config from 'config';
-import { decodeEntities } from 'lib/formatting';
 
 const debug = debugFactory( 'calypso:themes' );
-let themeDetailsCache = new Map();
-
-export function makeElement( ThemesComponent, Head, store, props ) {
-	return (
-		<Head
-			title={ props.title }
-			description={ props.description }
-			type={ 'website' }
-			canonicalUrl={ props.canonicalUrl }
-			image={ props.image }
-			tier={ props.tier || 'all' }>
-			<ThemesComponent { ...omit( props, [ 'title' ] ) } />
-		</Head>
-	);
-}
+const themeDetailsCache = new Map();
 
 export function fetchThemeDetailsData( context, next ) {
 	if ( ! config.isEnabled( 'manage/themes/details' ) || ! context.isServerSide ) {
@@ -76,24 +58,6 @@ export function fetchThemeDetailsData( context, next ) {
 export function details( context, next ) {
 	const { slug } = context.params;
 	const user = getCurrentUser( context.store.getState() );
-	const themeDetails = getThemeDetails( context.store.getState(), slug ) || false;
-	const themeName = themeDetails.name;
-	const title = i18n.translate( '%(themeName)s Theme', {
-		args: { themeName }
-	} );
-	const Head = user
-		? require( 'layout/head' )
-		: require( 'my-sites/themes/head' );
-
-	const props = {
-		themeSlug: slug,
-		contentSection: context.params.section,
-		title: decodeEntities( title ) + ' — WordPress.com', // TODO: Use lib/screen-title's buildTitle. Cf. https://github.com/Automattic/wp-calypso/issues/3796
-		description: decodeEntities( themeDetails.description ),
-		canonicalUrl: `https://wordpress.com/theme/${ slug }`, // TODO: use getDetailsUrl() When it becomes availavle
-		image: themeDetails.screenshot,
-		isLoggedIn: !! user
-	};
 
 	if ( startsWith( context.prevPath, '/design' ) ) {
 		context.store.dispatch( setBackPath( context.prevPath ) );
@@ -105,7 +69,7 @@ export function details( context, next ) {
 		</ThemeDetailsComponent>
 	);
 
-	context.primary = makeElement( ConnectedComponent, Head, context.store, props );
+	context.primary = ConnectedComponent( { themeSlug: slug, contentSection: context.params.section, isLoggedIn: !! user } );
 	context.secondary = null; // When we're logged in, we need to remove the sidebar.
 	next();
 }
diff --git a/client/my-sites/theme/main.jsx b/client/my-sites/theme/main.jsx
@@ -45,6 +45,8 @@ import { getBackPath } from 'state/themes/themes-ui/selectors';
 import EmptyContentComponent from 'components/empty-content';
 import ThemePreview from 'my-sites/themes/theme-preview';
 import PageViewTracker from 'lib/analytics/page-view-tracker';
+import Head from 'layout/head';
+import { decodeEntities } from 'lib/formatting';
 
 const ThemeSheet = React.createClass( {
 	displayName: 'ThemeSheet',
@@ -73,7 +75,7 @@ const ThemeSheet = React.createClass( {
 			label: React.PropTypes.string.isRequired,
 			action: React.PropTypes.func,
 			getUrl: React.PropTypes.func,
-		} ).isRequired,
+		} ),
 		secondaryOption: React.PropTypes.shape( {
 			label: React.PropTypes.string.isRequired,
 			action: React.PropTypes.func,
@@ -345,33 +347,48 @@ const ThemeSheet = React.createClass( {
 		const analyticsPath = `/theme/:slug${ section ? '/' + section : '' }${ siteID ? '/:site_id' : '' }`;
 		const analyticsPageTitle = `Themes > Details Sheet${ section ? ' > ' + titlecase( section ) : '' }${ siteID ? ' > Site' : '' }`;
 
+		const { name: themeName, description } = this.props;
+		const title = i18n.translate( '%(themeName)s Theme', {
+			args: { themeName }
+		} ); // TODO: Use lib/screen-title's buildTitle. Cf. https://github.com/Automattic/wp-calypso/issues/3796
+
+		const canonicalUrl = `https://wordpress.com/theme/${ this.props.id }`; // TODO: use getDetailsUrl() When it becomes availavle
+
 		return (
-			<Main className="theme__sheet">
-			<PageViewTracker path={ analyticsPath } title={ analyticsPageTitle }/>
-				{ this.renderBar() }
-				{ siteID && <QueryCurrentTheme siteId={ siteID }/> }
-				<ThanksModal
-					site={ this.props.selectedSite }
-					source={ 'details' }/>
-				{ this.state.showPreview && this.renderPreview() }
-				<HeaderCake className="theme__sheet-action-bar"
-							backHref={ this.props.backPath }
-							backText={ i18n.translate( 'All Themes' ) }>
-					{ this.renderButton() }
-				</HeaderCake>
-				<div className="theme__sheet-columns">
-					<div className="theme__sheet-column-left">
-						<div className="theme__sheet-content">
-							{ this.renderSectionNav( section ) }
-							{ this.renderSectionContent( section ) }
-							<div className="theme__sheet-footer-line"><Gridicon icon="my-sites" /></div>
+
+			<Head
+				title= { themeName && decodeEntities( title ) + ' — WordPress.com' }
+				description={ description && decodeEntities( description ) }
+				type={ 'website' }
+				canonicalUrl={ canonicalUrl }
+				image={ this.props.screenshot }>
+				<Main className="theme__sheet">
+					<PageViewTracker path={ analyticsPath } title={ analyticsPageTitle }/>
+						{ this.renderBar() }
+						{ siteID && <QueryCurrentTheme siteId={ siteID }/> }
+					<ThanksModal
+						site={ this.props.selectedSite }
+						source={ 'details' }/>
+					{ this.state.showPreview && this.renderPreview() }
+					<HeaderCake className="theme__sheet-action-bar"
+								backHref={ this.props.backPath }
+								backText={ i18n.translate( 'All Themes' ) }>
+						{ this.renderButton() }
+					</HeaderCake>
+					<div className="theme__sheet-columns">
+						<div className="theme__sheet-column-left">
+							<div className="theme__sheet-content">
+								{ this.renderSectionNav( section ) }
+								{ this.renderSectionContent( section ) }
+								<div className="theme__sheet-footer-line"><Gridicon icon="my-sites" /></div>
+							</div>
+						</div>
+						<div className="theme__sheet-column-right">
+							{ this.renderScreenshot() }
 						</div>
 					</div>
-					<div className="theme__sheet-column-right">
-						{ this.renderScreenshot() }
-					</div>
-				</div>
-			</Main>
+				</Main>
+			</Head>
 		);
 	},
 

diff --git a/client/my-sites/themes/controller.jsx b/client/my-sites/themes/controller.jsx
@@ -1,7 +1,9 @@
 /**
  * External Dependencies
  */
+import React from 'react';
 import i18n from 'i18n-calypso';
+import omit from 'lodash/omit';
 
 /**
  * Internal Dependencies
@@ -12,7 +14,20 @@ import LoggedOutComponent from './logged-out';
 import trackScrollPage from 'lib/track-scroll-page';
 import buildTitle from 'lib/screen-title/utils';
 import { getAnalyticsData } from './helpers';
-import { makeElement } from 'my-sites/theme/controller';
+
+function makeElement( ThemesComponent, Head, store, props ) {
+	return (
+		<Head
+			title={ props.title }
+			description={ props.description }
+			type={ 'website' }
+			canonicalUrl={ props.canonicalUrl }
+			image={ props.image }
+			tier={ props.tier || 'all' }>
+			<ThemesComponent { ...omit( props, [ 'title' ] ) } />
+		</Head>
+	);
+}
 
 function getProps( context ) {
 	const { tier, filter, site_id: siteId } = context.params;

diff --git a/client/my-sites/themes/head.jsx b/client/my-sites/themes/head.jsx
@@ -16,7 +16,7 @@ const ThemesHead = ( { title, description, canonicalUrl, type, image, tier, chil
 		description={ description ? description : get( 'description', tier ) }
 		canonicalUrl={ canonicalUrl ? canonicalUrl : get( 'canonicalUrl', tier ) }
 		type={ type ? type : 'website' }
-		site_name={ 'WordPress.com' }
+		siteName={ 'WordPress.com' }
 		image={ image ? image : {} } >
 		{ children }
 	</Head>

diff --git a/server/pragma-checker/index.js b/server/pragma-checker/index.js
@@ -13,6 +13,7 @@ var SSR_READY = '/** @ssr-ready **/';
 var IGNORED_MODULES = [
 	'config', // Different modules on client & server
 	'lib/wp', // Different modules on client & server
+	'lib/formatting', // Different modules on client & server
 	'lib/analytics', // nooped on the server until we develop an isomorphic version
 	'lib/route', // nooped on the server until we can extract the isomorphic bits
 	'lib/upgrades/actions', // nooped on the server as it still uses the singleton Flux architecture