Skip to content

Commit 97c3993

Browse files
[BUGFIX lts] Workaround for integer literals
Workaround for the Glimmer VM bug which encodes/decodes integer literals correctly. See #15635. Based on #18484 and #18484 (comment) Fixes #15635. Closes #18484. Co-authored-by: Saravana Kumar V <[email protected]> (cherry picked from commit 69fc6dd)
1 parent 55ffe41 commit 97c3993

File tree

5 files changed

+238
-59
lines changed

5 files changed

+238
-59
lines changed
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { assert } from '@ember/debug';
2+
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
3+
import { InternalHelperReference } from '../utils/references';
4+
5+
function i({ positional }: CapturedArguments): number {
6+
assert('[BUG] -i takes a single string', typeof positional.at(0).value() === 'string');
7+
return parseInt(positional.at(0).value() as string, 10);
8+
}
9+
10+
export default function(_vm: VM, args: Arguments) {
11+
return new InternalHelperReference(i, args.capture());
12+
}

packages/@ember/-internals/glimmer/lib/resolver.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { TemplateOnlyComponentDefinition } from './component-managers/template-o
2929
import { isHelperFactory, isSimpleHelper } from './helper';
3030
import { default as componentAssertionHelper } from './helpers/-assert-implicit-component-helper-argument';
3131
import { default as classHelper } from './helpers/-class';
32+
import { default as parseIntHelper } from './helpers/-i';
3233
import { default as inputTypeHelper } from './helpers/-input-type';
3334
import { default as normalizeClassHelper } from './helpers/-normalize-class';
3435
import { default as action } from './helpers/action';
@@ -254,6 +255,7 @@ const BUILTINS_HELPERS: IBuiltInHelpers = {
254255
unless: inlineUnless,
255256
'-class': classHelper,
256257
'-each-in': eachIn,
258+
'-i': parseIntHelper,
257259
'-input-type': inputTypeHelper,
258260
'-normalize-class': normalizeClassHelper,
259261
'-get-dynamic-var': getDynamicVar,

packages/@ember/-internals/glimmer/tests/integration/content-test.js

Lines changed: 135 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -10,61 +10,149 @@ import { HAS_NATIVE_SYMBOL } from '@ember/-internals/utils';
1010
import { constructStyleDeprecationMessage } from '@ember/-internals/views';
1111
import { Component, SafeString, htmlSafe } from '../utils/helpers';
1212

13-
moduleFor(
14-
'Static content tests',
15-
class extends RenderingTestCase {
16-
['@test it can render a static text node']() {
17-
this.render('hello');
18-
let text1 = this.assertTextNode(this.firstChild, 'hello');
13+
const EMPTY = Object.freeze({});
1914

20-
runTask(() => this.rerender());
15+
const LITERALS = [
16+
['foo', 'foo', '"foo"'],
17+
[undefined, EMPTY],
18+
[null, EMPTY],
19+
[true, 'true'],
20+
[false, 'false'],
21+
[0, '0'],
22+
[-0, '0', '-0'],
23+
[1, '1'],
24+
[-1, '-1'],
25+
[0.0, '0', '0.0'],
26+
[0.5, '0.5', '0.5'],
27+
[0.5, '0.5', '0.500000000000000000000000000000'],
28+
29+
// Kris Selden: that is a good one because it is above that 3 bit area,
30+
// but the shifted < 0 check doesn't return true:
31+
// https://github.com/glimmerjs/glimmer-vm/blob/761e78b2bef5de8b9b19ae5fb296380c21959ef8/packages/%40glimmer/opcode-compiler/lib/opcode-builder/encoder.ts#L277
32+
[536870912, '536870912'],
33+
34+
// Kris Selden: various other 10000000 and 1111111 combos
35+
[4294967296, '4294967296'],
36+
[4294967295, '4294967295'],
37+
[4294967294, '4294967294'],
38+
[536870913, '536870913'],
39+
[536870911, '536870911'],
40+
[268435455, '268435455'],
41+
];
42+
43+
let i = Number.MAX_SAFE_INTEGER;
44+
45+
while (i > 1) {
46+
LITERALS.push([i, `${i}`, `${i}`]);
47+
i = Math.round(i / 2);
48+
}
2149

22-
let text2 = this.assertTextNode(this.firstChild, 'hello');
50+
i = Number.MIN_SAFE_INTEGER;
2351

24-
this.assertSameNode(text1, text2);
25-
}
52+
while (i < -1) {
53+
LITERALS.push([i, `${i}`, `${i}`]);
54+
i = Math.round(i / 2);
55+
}
2656

27-
['@test it can render a static element']() {
28-
this.render('<p>hello</p>');
29-
let p1 = this.assertElement(this.firstChild, { tagName: 'p' });
30-
let text1 = this.assertTextNode(this.firstChild.firstChild, 'hello');
57+
class StaticContentTest extends RenderingTestCase {
58+
['@test it can render a static text node']() {
59+
this.render('hello');
60+
let text1 = this.assertTextNode(this.firstChild, 'hello');
3161

32-
runTask(() => this.rerender());
62+
runTask(() => this.rerender());
3363

34-
let p2 = this.assertElement(this.firstChild, { tagName: 'p' });
35-
let text2 = this.assertTextNode(this.firstChild.firstChild, 'hello');
64+
let text2 = this.assertTextNode(this.firstChild, 'hello');
3665

37-
this.assertSameNode(p1, p2);
38-
this.assertSameNode(text1, text2);
39-
}
66+
this.assertSameNode(text1, text2);
67+
}
4068

41-
['@test it can render a static template']() {
42-
let template = `
43-
<div class="header">
44-
<h1>Welcome to Ember.js</h1>
45-
</div>
46-
<div class="body">
47-
<h2>Why you should use Ember.js?</h2>
48-
<ol>
49-
<li>It's great</li>
50-
<li>It's awesome</li>
51-
<li>It's Ember.js</li>
52-
</ol>
53-
</div>
54-
<div class="footer">
55-
Ember.js is free, open source and always will be.
56-
</div>
57-
`;
69+
['@test it can render a static element']() {
70+
this.render('<p>hello</p>');
71+
let p1 = this.assertElement(this.firstChild, { tagName: 'p' });
72+
let text1 = this.assertTextNode(this.firstChild.firstChild, 'hello');
5873

59-
this.render(template);
60-
this.assertHTML(template);
74+
runTask(() => this.rerender());
6175

62-
runTask(() => this.rerender());
76+
let p2 = this.assertElement(this.firstChild, { tagName: 'p' });
77+
let text2 = this.assertTextNode(this.firstChild.firstChild, 'hello');
6378

64-
this.assertHTML(template);
65-
}
79+
this.assertSameNode(p1, p2);
80+
this.assertSameNode(text1, text2);
6681
}
67-
);
82+
83+
['@test it can render a static template']() {
84+
let template = `
85+
<div class="header">
86+
<h1>Welcome to Ember.js</h1>
87+
</div>
88+
<div class="body">
89+
<h2>Why you should use Ember.js?</h2>
90+
<ol>
91+
<li>It's great</li>
92+
<li>It's awesome</li>
93+
<li>It's Ember.js</li>
94+
</ol>
95+
</div>
96+
<div class="footer">
97+
Ember.js is free, open source and always will be.
98+
</div>
99+
`;
100+
101+
this.render(template);
102+
this.assertHTML(template);
103+
104+
runTask(() => this.rerender());
105+
106+
this.assertHTML(template);
107+
}
108+
}
109+
110+
class StaticContentTestGenerator {
111+
constructor(cases, tag = '@test') {
112+
this.cases = cases;
113+
this.tag = tag;
114+
}
115+
116+
generate([value, expected, label]) {
117+
let tag = this.tag;
118+
label = label || value;
119+
120+
return {
121+
[`${tag} rendering {{${label}}}`]() {
122+
this.render(`{{${label}}}`);
123+
124+
if (expected === EMPTY) {
125+
this.assertHTML('');
126+
} else {
127+
this.assertHTML(expected);
128+
}
129+
130+
this.assertStableRerender();
131+
},
132+
133+
[`${tag} rendering {{to-js ${label}}}`](assert) {
134+
this.registerHelper('to-js', ([actual]) => {
135+
assert.strictEqual(actual, value);
136+
return actual;
137+
});
138+
139+
this.render(`{{to-js ${label}}}`);
140+
141+
if (expected === EMPTY) {
142+
this.assertHTML('');
143+
} else {
144+
this.assertHTML(expected);
145+
}
146+
147+
this.assertStableRerender();
148+
},
149+
};
150+
}
151+
}
152+
153+
applyMixins(StaticContentTest, new StaticContentTestGenerator(LITERALS));
154+
155+
moduleFor('Static content tests', StaticContentTest);
68156

69157
class DynamicContentTest extends RenderingTestCase {
70158
/* abstract */
@@ -588,9 +676,7 @@ class DynamicContentTest extends RenderingTestCase {
588676
}
589677
}
590678

591-
const EMPTY = {};
592-
593-
class ContentTestGenerator {
679+
class DynamicContentTestGenerator {
594680
constructor(cases, tag = '@test') {
595681
this.cases = cases;
596682
this.tag = tag;
@@ -639,18 +725,8 @@ class ContentTestGenerator {
639725
}
640726
}
641727

642-
const SharedContentTestCases = new ContentTestGenerator([
643-
['foo', 'foo'],
644-
[0, '0'],
645-
[-0, '0', '-0'],
646-
[1, '1'],
647-
[-1, '-1'],
648-
[0.0, '0', '0.0'],
649-
[0.5, '0.5'],
650-
[undefined, EMPTY],
651-
[null, EMPTY],
652-
[true, 'true'],
653-
[false, 'false'],
728+
const SharedContentTestCases = new DynamicContentTestGenerator([
729+
...LITERALS,
654730
[NaN, 'NaN'],
655731
[new Date(2000, 0, 1), String(new Date(2000, 0, 1)), 'a Date object'],
656732
[Infinity, 'Infinity'],
@@ -679,7 +755,7 @@ const SharedContentTestCases = new ContentTestGenerator([
679755
['<b>Max</b><b>James</b>', '<b>Max</b><b>James</b>'],
680756
]);
681757

682-
let GlimmerContentTestCases = new ContentTestGenerator([
758+
let GlimmerContentTestCases = new DynamicContentTestGenerator([
683759
[Object.create(null), EMPTY, 'an object with no toString'],
684760
]);
685761

packages/ember-template-compiler/lib/plugins/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import AssertLocalVariableShadowingHelperInvocation from './assert-local-variabl
44
import AssertReservedNamedArguments from './assert-reserved-named-arguments';
55
import AssertSplattributeExpressions from './assert-splattribute-expression';
66
import DeprecateSendAction from './deprecate-send-action';
7+
import SafeIntegersBugfix from './safe-integers-bugfix';
78
import TransformActionSyntax from './transform-action-syntax';
89
import TransformAttrsIntoArgs from './transform-attrs-into-args';
910
import TransformComponentInvocation from './transform-component-invocation';
@@ -34,6 +35,7 @@ const transforms: Array<APluginFunc> = [
3435
TransformInElement,
3536
AssertIfHelperWithoutArguments,
3637
AssertSplattributeExpressions,
38+
SafeIntegersBugfix,
3739
];
3840

3941
if (SEND_ACTION) {
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax';
2+
import { MustacheStatement, NumberLiteral } from '@glimmer/syntax/dist/types/lib/types/nodes';
3+
4+
/**
5+
@module ember
6+
*/
7+
8+
/**
9+
A Glimmer2 AST transformation that replaces all instances of
10+
11+
```handlebars
12+
{{987654321}}
13+
```
14+
15+
to
16+
17+
```handlebars
18+
{{-i "987654321"}}
19+
```
20+
21+
as well as other integer number literals in sexp arguments, etc.
22+
23+
The version of Glimmer VM we are using has a bug that encodes
24+
certain integers incorrectly. This forces them into strings and
25+
use `{{-i}}` (which is a wrapper around `parseInt`) to decode
26+
them manually as a workaround.
27+
28+
This should be removed when the Glimmer VM bug is fixed.
29+
30+
@private
31+
@class SafeIntegersBugfix
32+
*/
33+
34+
export default function safeIntegersBugfix(env: ASTPluginEnvironment): ASTPlugin {
35+
let { builders: b } = env.syntax;
36+
37+
return {
38+
name: 'safe-integers-bugfix',
39+
40+
visitor: {
41+
MustacheStatement(node: AST.MustacheStatement): AST.MustacheStatement | undefined {
42+
if (!requiresWorkaround(node)) {
43+
return;
44+
}
45+
46+
return b.mustache(
47+
'-i',
48+
[b.string(String(node.path.value))],
49+
undefined,
50+
!node.escaped,
51+
node.loc
52+
);
53+
},
54+
55+
NumberLiteral(node: AST.NumberLiteral): AST.SubExpression | undefined {
56+
if (!requiresWorkaround(node)) {
57+
return;
58+
}
59+
60+
return b.sexpr('-i', [b.string(String(node.value))], undefined, node.loc);
61+
},
62+
},
63+
};
64+
}
65+
66+
type NumberLiteralMustacheStatement = MustacheStatement & { path: NumberLiteral };
67+
68+
function requiresWorkaround(node: AST.MustacheStatement): node is NumberLiteralMustacheStatement;
69+
function requiresWorkaround(node: AST.NumberLiteral): boolean;
70+
function requiresWorkaround(node: AST.MustacheStatement | AST.NumberLiteral): boolean {
71+
if (node.type === 'MustacheStatement' && node.path.type === 'NumberLiteral') {
72+
return requiresWorkaround(node.path);
73+
} else if (node.type === 'NumberLiteral') {
74+
return isInteger(node.value) && isOverflowing(node.value);
75+
} else {
76+
return false;
77+
}
78+
}
79+
80+
// Number.isInteger polyfill
81+
function isInteger(value: number): boolean {
82+
return isFinite(value) && Math.floor(value) === value;
83+
}
84+
85+
function isOverflowing(value: number): boolean {
86+
return value >= 2 ** 28;
87+
}

0 commit comments

Comments
 (0)