Stop using comments as boundary markers in dangerouslyRenderMarkup.
Jordan warned (and StackOverflow confirmed) that IE8 doesn't respect HTML comment nodes when setting `.innerHTML`: http://stackoverflow.com/questions/15006001/inserting-a-comment-in-innerhtml The virtue of the comment strategy was that the parser did all the work of maintaining the boundaries between markup chunks, using comments as the boundary markers. The new strategy (of tagging the first node with a special attribute) has a similar virtue, since the parser should preserve that attribute only for the nodes we care about, and any rendered nodes that do not have the attribute can be ignored (and complained about by `console.error`).
Ben Newman committed
Sep 9, 2013 at 14:59 UTC
888cc309e09ebc6bde7e13a30521ba5a47f217a2
2 files changed
+38
-82
src/dom/Danger.js
+35
-77
@@ -29,11 +29,8 @@ var getMarkupWrap = require('getMarkupWrap');
29
var invariant = require('invariant');
30
var mutateHTMLNodeWithMarkup = require('mutateHTMLNodeWithMarkup');
31
32
-var COMMENT_NODE_TYPE = 8;
33
-
34
-// This buffer will be reused in dangerouslyRenderMarkup to avoid unnecessary
35
-// array allocations.
36
-var reusableBuffer = [];
32
+var OPEN_TAG_NAME_EXP = /^(<[^ \/>]+)/;
33
+var RESULT_INDEX_ATTR = 'data-danger-index';
34
35
/**
36
* Extracts the `nodeName` from a string of markup.
@@ -89,97 +86,58 @@ var Danger = {
86
}
87
var markupListByNodeName = markupByNodeName[nodeName];
88
92
- var commentedMarkupList = reusableBuffer;
93
- commentedMarkupList.length = 0;
94
-
89
// This for-in loop skips the holes of the sparse array. The order of
90
// iteration should follow the order of assignment, which happens to match
91
// numerical index order, but we don't rely on that.
92
for (var resultIndex in markupListByNodeName) {
93
if (markupListByNodeName.hasOwnProperty(resultIndex)) {
100
- // Push the requested markup followed by a sentinel HTML
101
- // comment. Comments are a good choice for sentinels because they can
102
- // appear as children of any HTML node.
103
- commentedMarkupList.push(
104
- markupListByNodeName[resultIndex],
105
- // This resultIndex will be parsed back out below.
106
- '<!--danger:', resultIndex, '-->'
94
+ var markup = markupListByNodeName[resultIndex];
95
+
96
+ // Push the requested markup with an additional RESULT_INDEX_ATTR
97
+ // attribute. If the markup does not start with a < character, it
98
+ // will be discarded below (with an appropriate console.error).
99
+ markupListByNodeName[resultIndex] = markup.replace(
100
+ OPEN_TAG_NAME_EXP,
101
+ // This index will be parsed back out below.
102
+ '$1 ' + RESULT_INDEX_ATTR + '="' + resultIndex + '" '
103
);
104
}
105
}
106
111
- // Render each group of markup with similar wrapping `nodeName`, with
112
- // sentinel comments interspersed.
107
+ // Render each group of markup with similar wrapping `nodeName`.
108
var renderNodes = createNodesFromMarkup(
114
- commentedMarkupList.join(''),
109
+ markupListByNodeName.join(''),
110
emptyFunction // Do nothing special with <script> tags.
111
);
112
118
- var renderBuffer = reusableBuffer;
119
- renderBuffer.length = 0;
120
-
113
for (i = 0; i < renderNodes.length; ++i) {
114
var renderNode = renderNodes[i];
123
- if (renderNode.nodeType !== COMMENT_NODE_TYPE) {
124
- // Append all non-comment nodes to the buffer.
125
- renderBuffer.push(renderNode);
126
-
127
- } else {
128
- var comment = renderNode.nodeValue.split(':');
129
- if (comment[0] === 'danger') {
130
- resultIndex = comment[1];
131
- invariant(
132
- !resultList.hasOwnProperty(resultIndex),
133
- 'Danger: Assigning to an already-occupied result index.'
134
- );
135
-
136
- // Take the first node no matter how many were rendered.
137
- resultList[resultIndex] = renderBuffer[0] || null;
138
-
139
- if (__DEV__) {
140
- if (renderBuffer.length !== 1) {
141
- var args = [
142
- "Danger: '" + markupListByNodeName[resultIndex] + "' " +
143
- "rendered as " + renderBuffer.length + " nodes instead of 1:"
144
- ];
145
-
146
- // Pass the nodes as arguments to console.error so that they can
147
- // be inspected in the browser.
148
- args.push.apply(args, renderBuffer);
149
-
150
- // Use console.error instead of throwing so that we don't
151
- // interfere with the rendering of other markup.
152
- console.error.apply(console, args);
153
- }
154
- }
155
-
156
- // Empty the buffer after assigning its contents to
157
- // resultList[resultIndex];
158
- renderBuffer.length = 0;
159
-
160
- // This should match resultList.length and markupList.length when
161
- // we're done.
162
- resultListAssignmentCount += 1;
163
-
164
- } else {
165
- // It's some other kind of comment that we didn't create. Weirder
166
- // things have happened.
167
- renderBuffer.push(renderNode);
168
- console.warn(
169
- 'Danger: Markup unexpectedly rendered a comment node:',
170
- '<!--' + renderNode.nodeValue + '-->.'
171
- );
172
- }
115
+ if (renderNode.hasAttribute &&
116
+ renderNode.hasAttribute(RESULT_INDEX_ATTR)) {
117
+
118
+ resultIndex = +renderNode.getAttribute(RESULT_INDEX_ATTR);
119
+ renderNode.removeAttribute(RESULT_INDEX_ATTR);
120
+
121
+ invariant(
122
+ !resultList.hasOwnProperty(resultIndex),
123
+ 'Danger: Assigning to an already-occupied result index.'
124
+ );
125
+
126
+ resultList[resultIndex] = renderNode;
127
+
128
+ // This should match resultList.length and markupList.length when
129
+ // we're done.
130
+ resultListAssignmentCount += 1;
131
+
132
+ } else if (__DEV__) {
133
+ console.error(
134
+ "Danger: Discarding unexpected node:",
135
+ renderNode
136
+ );
137
}
138
}
139
}
140
177
- invariant(
178
- reusableBuffer.length === 0,
179
- 'Danger: Rendered malformed markup: %s.',
180
- reusableBuffer
181
- );
182
-
141
// Although resultList was populated out of order, it should now be a dense
142
// array.
143
invariant(
src/dom/__tests__/Danger-test.js
+3
-5
@@ -106,11 +106,9 @@ describe('Danger', function() {
106
107
expect(console.error.argsForCall.length).toBe(1);
108
109
- expect(args.length).toBe(3);
110
- expect(args[0]).toBe(
111
- "Danger: '<p></p><p></p>' rendered as 2 nodes instead of 1:");
112
- expect(args[1]).toBe(renderedMarkup[0]);
113
- expect(args[2].nodeName).toBe('P');
109
+ expect(args.length).toBe(2);
110
+ expect(args[0]).toBe('Danger: Discarding unexpected node:');
111
+ expect(args[1].nodeName).toBe('P');
112
});
113
});
114