Skip to content

Commit 71f8e2a

Browse files
committed
Switch to WeakMap with String objects to fix memory leak
- Replace LRU cache with WeakMap approach from PR developit#43 - Use String objects as WeakMap keys for automatic garbage collection - Memory bounded to active rendering context, no manual management needed - Update tests to use valueOf() to convert String objects to primitives Fixes developit#20, developit#34 Based on: developit#43
1 parent fdf3cf7 commit 71f8e2a

3 files changed

Lines changed: 35 additions & 39 deletions

File tree

package.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,5 @@
6969
"rollup-plugin-babel": "^2.4.0",
7070
"rollup-plugin-es3": "^1.0.3",
7171
"uglify-js": "^2.6.2"
72-
},
73-
"dependencies": {
74-
"lru-cache": "11.2.2"
7572
}
7673
}

src/vhtml.js

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import emptyTags from './empty-tags.js';
2-
import { LRUCache } from 'lru-cache';
32

43
// escape an attribute
54
let esc = str => String(str).replace(/[&<>"']/g, s=>`&${map[s]};`);
@@ -10,15 +9,10 @@ let DOMAttributeNames = {
109
htmlFor: 'for'
1110
};
1211

13-
// Use LRU cache to prevent unbounded memory growth
14-
// Based on heap analysis: ~5,400 strings per feed, 15s regeneration cycle
15-
// 10,000 max = ~2 feeds worth (allows cache hits during regeneration)
16-
// Strings are ~32 bytes each, so 10,000 entries = ~320 KB
17-
let sanitized = new LRUCache({
18-
max: 10000,
19-
maxSize: 5 * 1024 * 1024, // 5 MB safety cap
20-
sizeCalculation: () => 1 // Count-based eviction (uniform string sizes)
21-
});
12+
// Use WeakMap to prevent unbounded memory growth
13+
// String objects are automatically garbage collected when no longer referenced
14+
// Memory is bounded to active rendering context, cleaned up automatically
15+
let sanitized = new WeakMap();
2216

2317
/** Hyperscript reviver that constructs a sanitized HTML string. */
2418
export default function h(name, attrs) {
@@ -64,6 +58,7 @@ export default function h(name, attrs) {
6458
s += name ? `</${name}>` : '';
6559
}
6660

67-
sanitized.set(s, true);
68-
return s;
61+
let res = new String(s);
62+
sanitized.set(res, true);
63+
return res;
6964
}

test/vhtml.js

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@ import { expect } from 'chai';
33
/** @jsx h */
44
/*global describe,it*/
55

6+
function valueOf(str) {
7+
return str.valueOf();
8+
}
9+
610
describe('vhtml', () => {
711
it('should stringify html', () => {
812
let items = ['one', 'two', 'three'];
9-
expect(
13+
expect(valueOf(
1014
<div class="foo">
1115
<h1>Hi!</h1>
1216
<p>Here is a list of {items.length} items:</p>
@@ -16,46 +20,46 @@ describe('vhtml', () => {
1620
)) }
1721
</ul>
1822
</div>
19-
).to.equal(
23+
)).to.equal(
2024
`<div class="foo"><h1>Hi!</h1><p>Here is a list of 3 items:</p><ul><li>one</li><li>two</li><li>three</li></ul></div>`
2125
);
2226
});
2327

2428
it('should sanitize children', () => {
25-
expect(
29+
expect(valueOf(
2630
<div>
2731
{ `<strong>blocked</strong>` }
2832
<em>allowed</em>
2933
</div>
30-
).to.equal(
34+
)).to.equal(
3135
`<div>&lt;strong&gt;blocked&lt;/strong&gt;<em>allowed</em></div>`
3236
);
3337
});
3438

3539
it('should sanitize attributes', () => {
36-
expect(
40+
expect(valueOf(
3741
<div onclick={`&<>"'`} />
38-
).to.equal(
42+
)).to.equal(
3943
`<div onclick="&amp;&lt;&gt;&quot;&apos;"></div>`
4044
);
4145
});
4246

4347
it('should not sanitize the "dangerouslySetInnerHTML" attribute, and directly set its `__html` property as innerHTML', () => {
44-
expect(
48+
expect(valueOf(
4549
<div dangerouslySetInnerHTML={{ __html: "<span>Injected HTML</span>" }} />
46-
).to.equal(
50+
)).to.equal(
4751
`<div><span>Injected HTML</span></div>`
4852
);
4953
});
5054

5155
it('should flatten children', () => {
52-
expect(
56+
expect(valueOf(
5357
<div>
5458
{[['a','b']]}
5559
<c>d</c>
5660
{['e',['f'],[['g']]]}
5761
</div>
58-
).to.equal(
62+
)).to.equal(
5963
`<div>ab<c>d</c>efg</div>`
6064
);
6165
});
@@ -70,7 +74,7 @@ describe('vhtml', () => {
7074
</li>
7175
);
7276

73-
expect(
77+
expect(valueOf(
7478
<div class="foo">
7579
<h1>Hi!</h1>
7680
<ul>
@@ -81,7 +85,7 @@ describe('vhtml', () => {
8185
)) }
8286
</ul>
8387
</div>
84-
).to.equal(
88+
)).to.equal(
8589
`<div class="foo"><h1>Hi!</h1><ul><li id="0"><h4>one</h4>This is item one!</li><li id="1"><h4>two</h4>This is item two!</li></ul></div>`
8690
);
8791
});
@@ -95,7 +99,7 @@ describe('vhtml', () => {
9599
</li>
96100
);
97101

98-
expect(
102+
expect(valueOf(
99103
<div class="foo">
100104
<h1>Hi!</h1>
101105
<ul>
@@ -106,7 +110,7 @@ describe('vhtml', () => {
106110
)) }
107111
</ul>
108112
</div>
109-
).to.equal(
113+
)).to.equal(
110114
`<div class="foo"><h1>Hi!</h1><ul><li><h4></h4></li><li><h4></h4></li></ul></div>`
111115
);
112116
});
@@ -121,7 +125,7 @@ describe('vhtml', () => {
121125
</li>
122126
);
123127

124-
expect(
128+
expect(valueOf(
125129
<div class="foo">
126130
<h1>Hi!</h1>
127131
<ul>
@@ -132,13 +136,13 @@ describe('vhtml', () => {
132136
)) }
133137
</ul>
134138
</div>
135-
).to.equal(
139+
)).to.equal(
136140
`<div class="foo"><h1>Hi!</h1><ul><li><h4></h4>This is item one!</li><li><h4></h4>This is item two!</li></ul></div>`
137141
);
138142
});
139143

140144
it('should support empty (void) tags', () => {
141-
expect(
145+
expect(valueOf(
142146
<div>
143147
<area />
144148
<base />
@@ -161,31 +165,31 @@ describe('vhtml', () => {
161165
<span />
162166
<p />
163167
</div>
164-
).to.equal(
168+
)).to.equal(
165169
`<div><area><base><br><col><command><embed><hr><img><input><keygen><link><meta><param><source><track><wbr><div></div><span></span><p></p></div>`
166170
);
167171
});
168172

169173
it('should handle special prop names', () => {
170-
expect(
174+
expect(valueOf(
171175
<div className="my-class" htmlFor="id" />
172-
).to.equal(
176+
)).to.equal(
173177
'<div class="my-class" for="id"></div>'
174178
);
175179
});
176180

177181
it('should support string fragments', () => {
178-
expect(
182+
expect(valueOf(
179183
h(null, null, "foo", "bar", "baz")
180-
).to.equal(
184+
)).to.equal(
181185
'foobarbaz'
182186
);
183187
});
184188

185189
it('should support element fragments', () => {
186-
expect(
190+
expect(valueOf(
187191
h(null, null, <p>foo</p>, <em>bar</em>, <div class="qqqqqq">baz</div>)
188-
).to.equal(
192+
)).to.equal(
189193
'<p>foo</p><em>bar</em><div class="qqqqqq">baz</div>'
190194
);
191195
});

0 commit comments

Comments
 (0)