Skip to content

Instantly share code, notes, and snippets.

@domenic

domenic/complete-repo.html

Created June 7, 2012 15:31
Show Gist options
  • Save domenic/2889463 to your computer and use it in GitHub Desktop.
Save domenic/2889463 to your computer and use it in GitHub Desktop.
Download ZIP
WinJS Bug
Raw
readme.md

There is a bug in WinJS that makes programmatically creating templates impossible. That is, the following code will never work:

WinJS.UI.setOptions(el.winControl, {
    itemTemplate: new WinJS.Binding.Template(itemTemplateEl)
});

This code is attempting (!) to set a winControl's itemTemplate property to a new WinJS.Binding.Template object. But, it internally calls the setRenderer method shown below (ui.js). Which has a horrible, horrible bug.

This means programmatically setting templates is impossible with WinJS. A more extensive example of how this might occur in a very natural app is in complete-repo.html.

As mentioned, the buggy code is shown below in ui.js. The specific line in question is

this.renderer = newRenderer.renderItem;

This exhibits a classic JavaScript mistake, which you see repeatedly in StackOverflow questions: not differentiating between methods and functions. In short, the WinJS team is treating newRenderer.renderItem as a function, when in fact it is a method. By simply assigning it, as you would a function, you lose an important piece of context: the this pointer for the method.

The fix is to properly extract the method as a method, using Function.prototype.bind:

this.renderer = newRenderer.renderItem.bind(newRenderer);

Fixing this would introduce no compatibility issues. There is a code path that currently never works, so nobody can possibly be using it. This change would make it work.

A runtime patch to fix this behavior is shown in fix.js. As with all such runtime patches, the process is fragile.

Such a fundamental mistake in JavaScript semantics (the difference between methods and functions) does not inspire confidence. And the fact that it's in such a basic piece of the WinJS library (templating!) makes me concerned about the test coverage of the library in general.

This makes me fear that Microsoft does not expect us to write "real" WinJS apps, but only small, single-page samples that wouldn't need the flexibility provided by e.g. programmatic template binding.

Raw
complete-repo.html
<!DOCTYPE html>
<!-- File > New Project > Templates > JavaScript > Blank App; replace default.html with this -->
<html>
<head>
<meta charset="utf-8" />
<title>App2</title>
<!-- WinJS references -->
<link href="//Microsoft.WinJS.1.0.RC/css/ui-dark.css" rel="stylesheet" />
<script src="//Microsoft.WinJS.1.0.RC/js/base.js"></script>
<script src="//Microsoft.WinJS.1.0.RC/js/ui.js"></script>
<!-- App2 references -->
<link href="/css/default.css" rel="stylesheet" />
<script src="/js/default.js"></script>
</head>
<body>
<div id="todo-list" data-win-control="WinJS.UI.ListView"></div>
<div id="list-template">
<p data-win-bind="innerText: text"></p>
</div>
<script>
// This is one script module.
(function () {
"use strict";
window.module1Exports = {};
var templateEl = document.getElementById("list-template");
window.module1Exports.template = new WinJS.Binding.Template(templateEl);
}());
</script>
<script>
// This is another module, which initializes the UI.
(function () {
"use strict";
window.module2Exports = {};
var listEl = document.getElementById("todo-list");
window.module2Exports.listControl = WinJS.UI.process(listEl).then(function () {
return listEl.winControl;
});
}());
</script>
<script>
// This is a module that contains the data layer
(function () {
"use strict";
window.module3Exports = {};
var todos = ["fix this bug", "convince Microsoft it's worthwhile"];
var todoObjects = todos.map(function (text) { return { text: text }; });
var todoDataSource = new WinJS.Binding.List(todoObjects).dataSource;
window.module3Exports.todoDataSource = todoDataSource;
}());
</script>
<script>
// This is the composition module, responsible for composing the results of the other modules.
(function () {
"use strict";
var template = window.module1Exports.template;
var listControl = window.module2Exports.listControl;
var todoDataSource = window.module3Exports.todoDataSource;
listControl.then(function (winControl) {
WinJS.UI.setOptions(winControl, {
itemTemplate: template,
itemDataSource: todoDataSource
});
});
}());
</script>
</body>
</html>
Raw
fix.js
// Include this script file somewhere in your project.
// (Is modifying WinJS code kosher, by App Store guidlines?)
// Make sure to continually check back on ui.js, in case there are ever upstream changes, and if so re-do this patch.
// Pulled from //Microsoft.WinJS.1.0.RC/js/ui.js line 19133
WinJS.UI._ElementsPool.prototype.setRenderer = function (newRenderer) {
if (!newRenderer) {
if (WinJS.validation) {
throw new WinJS.ErrorFromName("WinJS.UI.ListView.invalidTemplate", WinJS.UI._strings.invalidTemplate);
}
this.renderer = WinJS.UI._trivialHtmlRenderer; // This is changed to get access to the default list renderer.
} else if (typeof newRenderer === "function") {
this.renderer = newRenderer;
} else if (typeof newRenderer === "object") {
if (WinJS.validation && !newRenderer.renderItem) {
throw new WinJS.ErrorFromName("WinJS.UI.ListView.invalidTemplate", WinJS.UI._strings.invalidTemplate);
}
this.renderer = newRenderer.renderItem.bind(newRenderer); // This is the other change (binding to newRenderer).
}
};
Raw
ui.js
// ui.js 1.0.RC line 19133
setRenderer: function (newRenderer) {
if (!newRenderer) {
if (WinJS.validation) {
throw new WinJS.ErrorFromName("WinJS.UI.ListView.invalidTemplate", WinJS.UI._strings.invalidTemplate);
}
this.renderer = trivialHtmlRenderer;
} else if (typeof newRenderer === "function") {
this.renderer = newRenderer;
} else if (typeof newRenderer === "object") {
if (WinJS.validation && !newRenderer.renderItem) {
throw new WinJS.ErrorFromName("WinJS.UI.ListView.invalidTemplate", WinJS.UI._strings.invalidTemplate);
}
this.renderer = newRenderer.renderItem; // NOTICE ANYTHING?!?!
}
},
@spankyj
Copy link

spankyj commented Jun 11, 2013

Josh from the WinJS team here. I'll admit that it is a little odd that you can't use a template in this fashion. The intention was that people would be binding to the template HTML element in the DOM (e.g. when you say data-win-options="{ itemTemplate: myTemplate }") and in that case it works out because we put a function on the Template's element which implements the renderItem contract.

In this case you can work around this by saying:

new WinJS.UI.ListView(element, { itemTemplate: new WinJS.Binding>Template(templateDiv).element });

-josh

@pke
Copy link

pke commented Mar 20, 2014

@spankyj sorry if I jump in here, I have a template related problem. Although its a declarative template.
I have an img element with an onerror handler in the template that hides the element if an error occurs during the image load
<img src="#" data-win-bind="src:brandLogo" onerror="this.style.display='none'"/>
However, all img elements are hidden when one of the listviews items cannot load its image.
Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment