Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Framework: Introduce AsyncLoad for asynchronous rendering by code splitting #5356

Closed
wants to merge 10 commits into from
Closed

Framework: Introduce AsyncLoad for asynchronous rendering by code splitting #5356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8dcf24d
Framework: Introduce CodeSplitRender for rendering
aduth May 12, 2016
f81431c
Rename component as AsyncLoad.
mtias Jul 1, 2016
ca56219
AsyncLoad: add placeholder render as fallback.
mtias Jul 1, 2016
15baa7a
Adjust default design of async-load.
mtias Jul 1, 2016
018eb3e
Editor: use AsyncLoad for drafts and author-selector.
mtias Jul 1, 2016
8c3c472
Editor: async load seo, loaction, and featured image panels.
mtias Jul 1, 2016
4a36949
Apply border radius to placeholder elements.
mtias Jul 4, 2016
e8f2ee6
Editor: wrap PostSchedule in async-load.
mtias Jul 4, 2016
c8a338f
Editor: wrap PostFormats in async-load.
mtias Jul 4, 2016
fe02a4f
Async load categories and tags in editor drawer.
mtias Jul 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assets/stylesheets/_components.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
@import 'auth/style';
@import 'components/accordion/style';
@import 'components/app-promo/style';
@import 'components/async-load/style';
@import 'components/author-selector/style';
@import 'components/bulk-select/style';
@import 'components/button/style';
Expand Down
39 changes: 39 additions & 0 deletions client/components/async-load/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* External dependencies
*/
import React, { Component, PropTypes } from 'react';
import omit from 'lodash/omit';

export default class AsyncLoad extends Component {
constructor() {
super( ...arguments );

this.state = {
component: null
};
}

componentWillMount() {
this.props.require( ( component ) => {
this.setState( { component } );
} );
}

render() {
if ( this.state.component ) {
const props = omit( this.props, Object.keys( this.constructor.propTypes ) );
return <this.state.component { ...props } />;
}

if ( this.props.placeholder ) {
return this.props.placeholder;
}

return <div className="async-load" />;
}
}

AsyncLoad.propTypes = {
require: PropTypes.func.isRequired,
placeholder: PropTypes.node
};
10 changes: 10 additions & 0 deletions client/components/async-load/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
.async-load {
background: lighten( $gray, 22% );
border-radius: 20px;
content: '';
display: block;
height: 8px;
width: 80px;
margin: 0 16px;
animation: pulse-light 0.8s ease-in-out infinite;
}
12 changes: 8 additions & 4 deletions client/post-editor/edit-post-status/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ const actions = require( 'lib/posts/actions' ),
Popover = require( 'components/popover' ),
InfoPopover = require( 'components/info-popover' ),
Tooltip = require( 'components/tooltip' ),
PostSchedule = require( 'components/post-schedule' ),
postScheduleUtils = require( 'components/post-schedule/utils' ),
siteUtils = require( 'lib/site/utils' ),
stats = require( 'lib/posts/stats' );

import AsyncLoad from 'components/async-load';

export default React.createClass( {
displayName: 'EditPostStatus',

Expand Down Expand Up @@ -190,12 +191,15 @@ export default React.createClass( {
onClose={ this.togglePostSchedulePopover }
>
<div className="edit-post-status__post-schedule">
<PostSchedule
<AsyncLoad
require={ function( callback ) {
require( [ 'components/post-schedule' ], callback );
} }
selectedDay={ selectedDay }
timezone={ tz }
gmtOffset={ gmt }
onDateChange={ this.props.onDateChange }>
</PostSchedule>
onDateChange={ this.props.onDateChange }
/>
</div>
</Popover>
);
Expand Down
5 changes: 5 additions & 0 deletions client/post-editor/edit-post-status/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,9 @@
display: block;
padding: 0 16px;
width: 237px;

.async-load {
margin: 16px 0;
width: auto;
}
}
14 changes: 11 additions & 3 deletions client/post-editor/editor-action-bar/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import React from 'react';
/**
* Internal dependencies
*/
import EditorAuthor from 'post-editor/editor-author';
import EditorDeletePost from 'post-editor/editor-delete-post';
import EditorPostType from 'post-editor/editor-post-type';
import EditorSticky from 'post-editor/editor-sticky';
Expand All @@ -16,6 +15,7 @@ import utils from 'lib/posts/utils';
import Tooltip from 'components/tooltip';
import Button from 'components/button';
import EditorActionBarViewLabel from './view-label';
import AsyncLoad from 'components/async-load';

export default React.createClass( {

Expand Down Expand Up @@ -58,7 +58,7 @@ export default React.createClass( {
};

return (
<EditorVisibility {...props} />
<EditorVisibility { ...props } />
);
},

Expand All @@ -68,7 +68,15 @@ export default React.createClass( {
return (
<div className="editor-action-bar">
<div className="editor-action-bar__first-group">
{ multiUserSite && <EditorAuthor post={ this.props.post } isNew={ this.props.isNew } /> }
{ multiUserSite &&
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-author' ], callback );
} }
post={ this.props.post }
isNew={ this.props.isNew }
/>
}
</div>
<EditorPostType />
<div className="editor-action-bar__last-group">
Expand Down
5 changes: 5 additions & 0 deletions client/post-editor/editor-action-bar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,8 @@
color: darken( $gray, 10% );
}
}

.editor-action-bar .async-load {
max-width: 30%;
margin-top: 6px;
}
37 changes: 37 additions & 0 deletions client/post-editor/editor-drawer/categories-and-tags.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* External dependencies
*/
import React from 'react';

/**
* Internal dependencies
*/
import CategoryListData from 'components/data/category-list-data';
import CategoriesTagsAccordion from 'post-editor/editor-categories-tags/accordion';
import TagListData from 'components/data/tag-list-data';

const EditorDrawerCategoriesAndTags = React.createClass( {

propTypes: {
site: React.PropTypes.object,
post: React.PropTypes.object
},

render() {
if ( ! this.props.site ) {
return;
}

return (
<CategoryListData siteId={ this.props.site.ID }>
<TagListData siteId={ this.props.site.ID }>
<CategoriesTagsAccordion
site={ this.props.site }
post={ this.props.post } />
</TagListData>
</CategoryListData>
);
}
} );

export default EditorDrawerCategoriesAndTags;
108 changes: 56 additions & 52 deletions client/post-editor/editor-drawer/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,9 @@ import { connect } from 'react-redux';
import Accordion from 'components/accordion';
import AccordionSection from 'components/accordion/section';
import Gridicon from 'components/gridicon';
import CategoriesTagsAccordion from 'post-editor/editor-categories-tags/accordion';
import CategoryListData from 'components/data/category-list-data';
import TagListData from 'components/data/tag-list-data';
import EditorSharingAccordion from 'post-editor/editor-sharing/accordion';
import AsyncLoad from 'components/async-load';
import FormTextarea from 'components/forms/form-textarea';
import PostFormatsData from 'components/data/post-formats-data';
import PostFormatsAccordion from 'post-editor/editor-post-formats/accordion';
import Location from 'post-editor/editor-location';
import Discussion from 'post-editor/editor-discussion';
import PageParent from 'post-editor/editor-page-parent';
import SeoAccordion from 'post-editor/editor-seo-accordion';
import EditorMoreOptionsSlug from 'post-editor/editor-more-options/slug';
import InfoPopover from 'components/info-popover';
import PageTemplatesData from 'components/data/page-templates-data';
Expand All @@ -39,7 +31,6 @@ import { getEditorPostId } from 'state/ui/editor/selectors';
import { getEditedPostValue } from 'state/posts/selectors';
import { getPostType } from 'state/post-types/selectors';
import config from 'config';
import EditorDrawerFeaturedImage from './featured-image';
import EditorDrawerTaxonomies from './taxonomies';

/**
Expand Down Expand Up @@ -112,38 +103,27 @@ const EditorDrawer = React.createClass( {
},

renderTaxonomies: function() {
var element;

if ( config.isEnabled( 'manage/custom-post-types' ) &&
! includes( [ 'post', 'page' ], this.props.type ) ) {
return (
<EditorDrawerTaxonomies
postTerms={ this.props.post && this.props.post.terms }
/>
);
}
return (
<EditorDrawerTaxonomies
postTerms={ this.props.post && this.props.post.terms }
/>
);
},

renderCategoriesAndTags: function() {
if ( ! this.currentPostTypeSupports( 'tags' ) ) {
return;
}

element = (
<CategoriesTagsAccordion
return (
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-drawer/categories-and-tags' ], callback );
} }
site={ this.props.site }
post={ this.props.post } />
post={ this.props.post }
/>
);

if ( this.props.site ) {
element = (
<CategoryListData siteId={ this.props.site.ID }>
<TagListData siteId={ this.props.site.ID }>
{ element }
</TagListData>
</CategoryListData>
);
}

return element;
},

renderPostFormats: function() {
Expand All @@ -153,18 +133,22 @@ const EditorDrawer = React.createClass( {
}

return (
<PostFormatsData siteId={ this.props.site.ID }>
<PostFormatsAccordion
site={ this.props.site }
post={ this.props.post }
className="editor-drawer__accordion" />
</PostFormatsData>
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-drawer/post-formats' ], callback );
} }
site={ this.props.site }
post={ this.props.post }
/>
);
},

renderSharing: function() {
return (
<EditorSharingAccordion
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-sharing/accordion' ], callback );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern is readability of the tree if we start having loader wrappers everywhere. Nothing jumps out at me immediately as possible improvements, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern is readability of the tree if we start having loader wrappers everywhere. Nothing jumps out at me immediately as possible improvements, though.

Yeah, it's an unfortunate consequence of Webpack only being capable of static analysis for the code splitting, since otherwise would offer something a bit more simplified like:

require="post-editor/editor-sharing/accordion"

...and have the component manage the complexities.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this work? As you stated, webpack only does static analysis, but this would imply that is can do dynamic analysis... i think. Usually webpack is using require.ensure to inject split points.. plain requires, even hidden in functions, should be treated as hard deps that need to be in the same chunk (or a parent chunk)..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually webpack is using require.ensure to inject split points

It's not entirely obvious, but this is the AMD (not CommonJS) syntax for asynchronously loading modules (note the callback). Both are supported by Webpack in defining a split point. The reason for using AMD is that it provides the loaded modules as arguments to the callback function, whereas CommonJS passes only require, and the CodeSplitRender function is otherwise not capable of determining which modules it should proceed to load (unless we duplicated the modules array as a separate prop).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhhhhhhh... subtle.

} }
site={ this.props.site }
post={ this.props.post }
isNew={ this.props.isNew } />
Expand All @@ -177,14 +161,18 @@ const EditorDrawer = React.createClass( {
}

return (
<EditorDrawerFeaturedImage
<AsyncLoad
require={ function( callback ) {
require( [ './featured-image' ], callback );
} }
site={ this.props.site }
post={ this.props.post } />
post={ this.props.post }
/>
);
},

renderExcerpt: function() {
var excerpt;
let excerpt;

if ( ! this.currentPostTypeSupports( 'excerpt' ) ) {
return;
Expand Down Expand Up @@ -228,7 +216,12 @@ const EditorDrawer = React.createClass( {
return (
<AccordionSection>
<span className="editor-drawer__label-text">{ this.translate( 'Location' ) }</span>
<Location coordinates={ PostMetadata.geoCoordinates( this.props.post ) } />
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-location' ], callback );
} }
coordinates={ PostMetadata.geoCoordinates( this.props.post ) }
/>
</AccordionSection>
);
},
Expand All @@ -238,9 +231,12 @@ const EditorDrawer = React.createClass( {
return;
}

return(
return (
<AccordionSection>
<Discussion
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-discussion' ], callback );
} }
site={ this.props.site }
post={ this.props.post }
isNew={ this.props.isNew }
Expand All @@ -261,7 +257,12 @@ const EditorDrawer = React.createClass( {
}

return (
<SeoAccordion metaDescription={ PostMetadata.metaDescription( this.props.post ) } />
<AsyncLoad
require={ function( callback ) {
require( [ 'post-editor/editor-seo-accordion' ], callback );
} }
metaDescription={ PostMetadata.metaDescription( this.props.post ) }
/>
);
},

Expand Down Expand Up @@ -302,7 +303,7 @@ const EditorDrawer = React.createClass( {
<Accordion
title={ this.translate( 'Page Options' ) }
icon={ <Gridicon icon="pages" /> }>
{ this.props.site && this.props.post ?
{ this.props.site && this.props.post &&
<div>
<PageParent siteId={ this.props.site.ID }
postId={ this.props.post.ID }
Expand All @@ -312,7 +313,7 @@ const EditorDrawer = React.createClass( {
<PageTemplates post={ this.props.post } />
</PageTemplatesData>
</div>
: null }
}
<PageOrder menuOrder={ this.props.post ? this.props.post.menu_order : 0 } />
</Accordion>
);
Expand All @@ -326,7 +327,10 @@ const EditorDrawer = React.createClass( {
{ site && ! this.hasHardCodedPostTypeSupports( type ) && (
<QueryPostTypes siteId={ site.ID } />
) }
{ this.renderTaxonomies() }
{ config.isEnabled( 'manage/custom-post-types' ) && ! includes( [ 'post', 'page' ], this.props.type )
Copy link
Contributor Author

@aduth aduth Jul 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tab in here. In fact, I think we should drop the ! includes part of the condition altogether, but fine to leave that as a separate task if you'd prefer.

Edit: As in, this.renderTaxonomies should be called for any post type (including post and pages), but renderCategoriesAndTags called for post and page only (above renderTaxonomies I assume)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I haven't moved taxonomies to be async-loaded because it was causing issues. Leaving that up to your or @timmyc's discretion :)

? this.renderTaxonomies()
: this.renderCategoriesAndTags()
}
{ this.renderFeaturedImage() }
{ this.renderPageOptions() }
{ this.renderSharing() }
Expand Down
Loading