Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Allow lazy fragment initializaiton through promise #94

Merged
merged 2 commits into from
Jan 4, 2017
Merged
Changes from 1 commit
Commits
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
51 changes: 39 additions & 12 deletions src/pipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var Pipe = (function (doc, perf) { //eslint-disable-line no-unused-vars, strict
return script;
}
}
};
}
function placeholder (index) {
placeholders[index] = currentScript();
}
Expand Down Expand Up @@ -46,21 +46,48 @@ var Pipe = (function (doc, perf) { //eslint-disable-line no-unused-vars, strict
start.parentNode.removeChild(start);
end.parentNode.removeChild(end);
script && require([script], function (i) {
// Exposed fragment initialization Function/Promise
var init = i && i.__esModule ? i.default : i;
if (typeof init === 'function') {
// capture initializaion cost of each fragment on the page
if (perf && 'mark' in perf) {
perf.mark(fragmentId);
// early return
if (!init) {
return;
}
// check if User Timing API is supported
var isUTSupported = perf && 'mark' in perf;

function isPromise(obj) {
Copy link
Contributor

Choose a reason for hiding this comment

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

pull this out into a library function and import it

Copy link
Collaborator Author

@vigneshshanmugam vigneshshanmugam Jan 3, 2017

Choose a reason for hiding this comment

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

I would keep it simple.. will probably even inline it than keeping it as fn

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe, but it's probably something that should be covered by a test, so pulling it out into a library func is going to help with that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah i didnt do that since its not used anywhere other than this one place..

Copy link
Contributor

Choose a reason for hiding this comment

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

oki

return typeof obj === 'object'
&& typeof obj.then === 'function';
}

function measureInitCost(fragmentId, metricName) {
if (!isUTSupported) {
return;
}
perf.mark(fragmentId + '-end');
perf.measure(metricName + fragmentId, fragmentId, fragmentId + '-end');
// Clear the perf entries buffer after measuring
perf.clearMarks(fragmentId);
perf.clearMarks(fragmentId + '-end');
}

(function executeAndCapture() {
isUTSupported && perf.mark(fragmentId);
// Check if the exposed function is a Promise to allow lazy rendering
// Capture initializaion cost of each fragment on the page using User Timing API if available
if (isPromise(init)) {
init.then(function(module) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that init function should rather return a promise

module(node);
measureInitCost(fragmentId, 'fragment-');
});
} else if (typeof init === 'function') {
init(node);
perf.mark(fragmentId + '-end');
perf.measure('fragment-' + fragmentId, fragmentId, fragmentId + '-end');
// Clear the perf entries buffer after measuring
perf.clearMarks(fragmentId);
perf.clearMarks(fragmentId + '-end');
measureInitCost(fragmentId, 'fragment-');
} else {
init(node);
// this case should be ideally caught by early return
isUTSupported && perf.clearMarks(fragmentId);
}
}
})();
});
}
/* @preserve - loadCSS: load a CSS file asynchronously. [c]2016 @scottjehl, Filament Group, Inc. Licensed MIT */
Expand Down