Skip to content

Commit

Permalink
Merge pull request #18730 from emberjs/integers-test
Browse files Browse the repository at this point in the history
Workaround for #15635 (integer encoding issue)
  • Loading branch information
chancancode committed Feb 12, 2020
2 parents e9e726c + 69fc6dd commit 1f08044
Show file tree
Hide file tree
Showing 5 changed files with 238 additions and 59 deletions.
12 changes: 12 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/-i.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { assert } from '@ember/debug';
import { CapturedArguments, VM, VMArguments } from '@glimmer/interfaces';
import { HelperRootReference } from '@glimmer/reference';

function i({ positional }: CapturedArguments): number {
assert('[BUG] -i takes a single string', typeof positional.at(0).value() === 'string');
return parseInt(positional.at(0).value() as string, 10);
}

export default function(args: VMArguments, vm: VM) {
return new HelperRootReference(i, args.capture(), vm.env);
}
2 changes: 2 additions & 0 deletions packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import InternalComponentManager, {
import { TemplateOnlyComponentDefinition } from './component-managers/template-only';
import { isHelperFactory, isSimpleHelper } from './helper';
import { default as componentAssertionHelper } from './helpers/-assert-implicit-component-helper-argument';
import { default as parseIntHelper } from './helpers/-i';
import { default as inputTypeHelper } from './helpers/-input-type';
import { default as normalizeClassHelper } from './helpers/-normalize-class';
import { default as trackArray } from './helpers/-track-array';
Expand Down Expand Up @@ -250,6 +251,7 @@ const BUILTINS_HELPERS: IBuiltInHelpers = {
unless: inlineUnless,
'-hash': hash,
'-each-in': eachIn,
'-i': parseIntHelper,
'-input-type': inputTypeHelper,
'-normalize-class': normalizeClassHelper,
'-track-array': trackArray,
Expand Down
194 changes: 135 additions & 59 deletions packages/@ember/-internals/glimmer/tests/integration/content-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,61 +10,149 @@ import { HAS_NATIVE_SYMBOL } from '@ember/-internals/utils';
import { constructStyleDeprecationMessage } from '@ember/-internals/views';
import { Component, SafeString, htmlSafe } from '../utils/helpers';

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

runTask(() => this.rerender());
const LITERALS = [
['foo', 'foo', '"foo"'],
[undefined, EMPTY],
[null, EMPTY],
[true, 'true'],
[false, 'false'],
[0, '0'],
[-0, '0', '-0'],
[1, '1'],
[-1, '-1'],
[0.0, '0', '0.0'],
[0.5, '0.5', '0.5'],
[0.5, '0.5', '0.500000000000000000000000000000'],

// Kris Selden: that is a good one because it is above that 3 bit area,
// but the shifted < 0 check doesn't return true:
// https://github.com/glimmerjs/glimmer-vm/blob/761e78b2bef5de8b9b19ae5fb296380c21959ef8/packages/%40glimmer/opcode-compiler/lib/opcode-builder/encoder.ts#L277
[536870912, '536870912'],

// Kris Selden: various other 10000000 and 1111111 combos
[4294967296, '4294967296'],
[4294967295, '4294967295'],
[4294967294, '4294967294'],
[536870913, '536870913'],
[536870911, '536870911'],
[268435455, '268435455'],
];

let i = Number.MAX_SAFE_INTEGER;

while (i > 1) {
LITERALS.push([i, `${i}`, `${i}`]);
i = Math.round(i / 2);
}

let text2 = this.assertTextNode(this.firstChild, 'hello');
i = Number.MIN_SAFE_INTEGER;

this.assertSameNode(text1, text2);
}
while (i < -1) {
LITERALS.push([i, `${i}`, `${i}`]);
i = Math.round(i / 2);
}

['@test it can render a static element']() {
this.render('<p>hello</p>');
let p1 = this.assertElement(this.firstChild, { tagName: 'p' });
let text1 = this.assertTextNode(this.firstChild.firstChild, 'hello');
class StaticContentTest extends RenderingTestCase {
['@test it can render a static text node']() {
this.render('hello');
let text1 = this.assertTextNode(this.firstChild, 'hello');

runTask(() => this.rerender());
runTask(() => this.rerender());

let p2 = this.assertElement(this.firstChild, { tagName: 'p' });
let text2 = this.assertTextNode(this.firstChild.firstChild, 'hello');
let text2 = this.assertTextNode(this.firstChild, 'hello');

this.assertSameNode(p1, p2);
this.assertSameNode(text1, text2);
}
this.assertSameNode(text1, text2);
}

['@test it can render a static template']() {
let template = `
<div class="header">
<h1>Welcome to Ember.js</h1>
</div>
<div class="body">
<h2>Why you should use Ember.js?</h2>
<ol>
<li>It's great</li>
<li>It's awesome</li>
<li>It's Ember.js</li>
</ol>
</div>
<div class="footer">
Ember.js is free, open source and always will be.
</div>
`;
['@test it can render a static element']() {
this.render('<p>hello</p>');
let p1 = this.assertElement(this.firstChild, { tagName: 'p' });
let text1 = this.assertTextNode(this.firstChild.firstChild, 'hello');

this.render(template);
this.assertHTML(template);
runTask(() => this.rerender());

runTask(() => this.rerender());
let p2 = this.assertElement(this.firstChild, { tagName: 'p' });
let text2 = this.assertTextNode(this.firstChild.firstChild, 'hello');

this.assertHTML(template);
}
this.assertSameNode(p1, p2);
this.assertSameNode(text1, text2);
}
);

['@test it can render a static template']() {
let template = `
<div class="header">
<h1>Welcome to Ember.js</h1>
</div>
<div class="body">
<h2>Why you should use Ember.js?</h2>
<ol>
<li>It's great</li>
<li>It's awesome</li>
<li>It's Ember.js</li>
</ol>
</div>
<div class="footer">
Ember.js is free, open source and always will be.
</div>
`;

this.render(template);
this.assertHTML(template);

runTask(() => this.rerender());

this.assertHTML(template);
}
}

class StaticContentTestGenerator {
constructor(cases, tag = '@test') {
this.cases = cases;
this.tag = tag;
}

generate([value, expected, label]) {
let tag = this.tag;
label = label || value;

return {
[`${tag} rendering {{${label}}}`]() {
this.render(`{{${label}}}`);

if (expected === EMPTY) {
this.assertHTML('');
} else {
this.assertHTML(expected);
}

this.assertStableRerender();
},

[`${tag} rendering {{to-js ${label}}}`](assert) {
this.registerHelper('to-js', ([actual]) => {
assert.strictEqual(actual, value);
return actual;
});

this.render(`{{to-js ${label}}}`);

if (expected === EMPTY) {
this.assertHTML('');
} else {
this.assertHTML(expected);
}

this.assertStableRerender();
},
};
}
}

applyMixins(StaticContentTest, new StaticContentTestGenerator(LITERALS));

moduleFor('Static content tests', StaticContentTest);

class DynamicContentTest extends RenderingTestCase {
/* abstract */
Expand Down Expand Up @@ -588,9 +676,7 @@ class DynamicContentTest extends RenderingTestCase {
}
}

const EMPTY = {};

class ContentTestGenerator {
class DynamicContentTestGenerator {
constructor(cases, tag = '@test') {
this.cases = cases;
this.tag = tag;
Expand Down Expand Up @@ -639,18 +725,8 @@ class ContentTestGenerator {
}
}

const SharedContentTestCases = new ContentTestGenerator([
['foo', 'foo'],
[0, '0'],
[-0, '0', '-0'],
[1, '1'],
[-1, '-1'],
[0.0, '0', '0.0'],
[0.5, '0.5'],
[undefined, EMPTY],
[null, EMPTY],
[true, 'true'],
[false, 'false'],
const SharedContentTestCases = new DynamicContentTestGenerator([
...LITERALS,
[NaN, 'NaN'],
[new Date(2000, 0, 1), String(new Date(2000, 0, 1)), 'a Date object'],
[Infinity, 'Infinity'],
Expand Down Expand Up @@ -679,7 +755,7 @@ const SharedContentTestCases = new ContentTestGenerator([
['<b>Max</b><b>James</b>', '<b>Max</b><b>James</b>'],
]);

let GlimmerContentTestCases = new ContentTestGenerator([
let GlimmerContentTestCases = new DynamicContentTestGenerator([
[Object.create(null), EMPTY, 'an object with no toString'],
]);

Expand Down
2 changes: 2 additions & 0 deletions packages/ember-template-compiler/lib/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import AssertLocalVariableShadowingHelperInvocation from './assert-local-variabl
import AssertReservedNamedArguments from './assert-reserved-named-arguments';
import AssertSplattributeExpressions from './assert-splattribute-expression';
import DeprecateSendAction from './deprecate-send-action';
import SafeIntegersBugfix from './safe-integers-bugfix';
import TransformActionSyntax from './transform-action-syntax';
import TransformAttrsIntoArgs from './transform-attrs-into-args';
import TransformComponentInvocation from './transform-component-invocation';
Expand Down Expand Up @@ -41,6 +42,7 @@ const transforms: Array<APluginFunc> = [
AssertSplattributeExpressions,
TransformEachTrackArray,
TransformWrapMountAndOutlet,
SafeIntegersBugfix,
];

if (SEND_ACTION) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { AST, ASTPlugin, ASTPluginEnvironment } from '@glimmer/syntax';
import { MustacheStatement, NumberLiteral } from '@glimmer/syntax/dist/types/lib/types/nodes';

/**
@module ember
*/

/**
A Glimmer2 AST transformation that replaces all instances of
```handlebars
{{987654321}}
```
to
```handlebars
{{-i "987654321"}}
```
as well as other integer number literals in sexp arguments, etc.
The version of Glimmer VM we are using has a bug that encodes
certain integers incorrectly. This forces them into strings and
use `{{-i}}` (which is a wrapper around `parseInt`) to decode
them manually as a workaround.
This should be removed when the Glimmer VM bug is fixed.
@private
@class SafeIntegersBugfix
*/

export default function safeIntegersBugfix(env: ASTPluginEnvironment): ASTPlugin {
let { builders: b } = env.syntax;

return {
name: 'safe-integers-bugfix',

visitor: {
MustacheStatement(node: AST.MustacheStatement): AST.MustacheStatement | undefined {
if (!requiresWorkaround(node)) {
return;
}

return b.mustache(
'-i',
[b.string(String(node.path.value))],
undefined,
!node.escaped,
node.loc
);
},

NumberLiteral(node: AST.NumberLiteral): AST.SubExpression | undefined {
if (!requiresWorkaround(node)) {
return;
}

return b.sexpr('-i', [b.string(String(node.value))], undefined, node.loc);
},
},
};
}

type NumberLiteralMustacheStatement = MustacheStatement & { path: NumberLiteral };

function requiresWorkaround(node: AST.MustacheStatement): node is NumberLiteralMustacheStatement;
function requiresWorkaround(node: AST.NumberLiteral): boolean;
function requiresWorkaround(node: AST.MustacheStatement | AST.NumberLiteral): boolean {
if (node.type === 'MustacheStatement' && node.path.type === 'NumberLiteral') {
return requiresWorkaround(node.path);
} else if (node.type === 'NumberLiteral') {
return isInteger(node.value) && isOverflowing(node.value);
} else {
return false;
}
}

// Number.isInteger polyfill
function isInteger(value: number): boolean {
return isFinite(value) && Math.floor(value) === value;
}

function isOverflowing(value: number): boolean {
return value >= 2 ** 28;
}

0 comments on commit 1f08044

Please sign in to comment.