Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC #338] Implement new template helpers #17177

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
16d6724
[RFC #338] Implement new template helpers
cibernox Nov 2, 2018
95925ab
Implement {{not}} helper
cibernox Nov 3, 2018
4fecacb
Implement `gt` helper
cibernox Nov 3, 2018
98e99e1
Implement {{tl}} helper
cibernox Nov 4, 2018
207ea8c
Implement {{gte}} and {{lte}} helpers
cibernox Nov 4, 2018
22a8d33
Implement {{and}} helper
cibernox Nov 4, 2018
7142979
Implement {{or}} helper
cibernox Nov 4, 2018
58f1426
Perf improvement: replace `[].reduce` with `for` loop
cibernox Nov 4, 2018
552f776
Ensure gt, get, lt, and lte compare treat null on the second argument…
cibernox Nov 4, 2018
d024e91
Implement {{not-eq}} helper
cibernox Nov 5, 2018
1d791ad
Fix linting problems
cibernox Nov 5, 2018
77fc37a
Add feature flag for new template helpers
cibernox Nov 6, 2018
544da6d
fix linting errors
locks Nov 6, 2018
b6f5f7f
Update packages/@ember/-internals/glimmer/lib/helpers/or.ts
locks Nov 6, 2018
c720549
Update packages/@ember/-internals/glimmer/lib/helpers/not.ts
locks Nov 6, 2018
4ccf90a
Update packages/@ember/-internals/glimmer/lib/helpers/not-eq.ts
locks Nov 6, 2018
8fbdda2
Update packages/@ember/-internals/glimmer/lib/helpers/gte.ts
locks Nov 6, 2018
9f8e8a5
Update packages/@ember/-internals/glimmer/lib/helpers/gt.ts
locks Nov 6, 2018
f3e4192
Update packages/@ember/-internals/glimmer/lib/helpers/eq.ts
locks Nov 6, 2018
81e61d7
Update packages/@ember/-internals/glimmer/lib/helpers/and.ts
locks Nov 6, 2018
1b7e613
Add @category ember-basic-template-helpers to docs of all new helpers
cibernox Nov 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,8 @@ for a detailed explanation.
* `ember-template-block-let-helper`

Introduce the block form of the `let` helper per [RFC](https://github.com/emberjs/rfcs/blob/78211b11387e7f477264e322687f1ec5ab131361/text/0286-block-let-template-helper.md).

* `ember-basic-template-helpers`

Introduces several template helpers to perform basic comparisons and boolean operation in templates:
`{{eq}}`, `{{not-eq}}`, `{{gt}}`, `{{gte}}`, `{{lt}}`, `{{lte}}`, `{{not}}`, `{{and}}` and `{{or}}`.
37 changes: 37 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/and.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Evaluates the given arguments with && in short-circuit

Example:

```handlebars
{{and isAdmin isIdle validData}}

{{! be validData if `isAdmin` and `isIdle` are both truthy}}
```

@public
@method and
@for Ember.Templates.helpers
@since 3.7.0
*/
function and({ positional: { references } }: CapturedArguments) {
let last: any = true;
for (let i = 0; i < references.length; i++) {
last = references[i].value();
if (!last) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chancancode - Should we consider [] as falsey here? In general, in template land I think we do right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I think we do.

export default function toBool(predicate: Opaque): boolean {
if (isArray(predicate)) {
return (predicate as { length: number }).length !== 0;
} else {
return !!predicate;
}
}

I bigger question is should we support proxies, I completely forgot about that. I think that needed to be discussed and clarified in the RFC 🤧

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is surprising. I was expecting and/or helpers to behave just like && / II. Why this deviation from standard JS behaviour?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{#each @posts as |post|}}
  ...
{{else}}
  No posts!
{{/each}}

Copy link
Contributor Author

@cibernox cibernox Nov 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{#each}} is designed to display lists, and seems normal that it has special behavior for empty lists, but and and or are not for lists, so I don't see why the behavior of {{each}} should influence them and make them behave different than && and ||

On top of that, this is not how ember-truth-helpers behave. I'm not saying that we can't deviate from what e-t-h if there is a good reason, but it would be nice not to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it really isn't about each, {{#if someEmptyArray}}{{else}}{{/if}} will render the falsey branch. I think it is very confusing to have if behave differently than eq/not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works the same way for...

{{#if @posts as |post|}}
  ...
{{else}}
  No posts!
{{/if}}

...and with, let etc. I think the bottom line is it would be pretty surprising if {{else}} doesn't work consistently in the control flow things in the "language".

So now what if you refactor from the first example into...

{{#each (or @posts @reblogs) as |post|}}
  ...
{{else}}
  No posts!
{{/each}}

...what should this do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is probably important for the "language" to have one boolean semantics, and for better or for worse empty arrays are falsey everywhere else.

return last;
}
}
return last;
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(and, args.capture());
}
30 changes: 30 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/eq.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with ===

Example:

```handlebars
{{eq type "button"}}

{{! be true if `type` is "button"}}
```

@public
@method eq
@for Ember.Templates.helpers
@since 3.7.0
*/
function eq({ positional: { references } }: CapturedArguments) {
return references[0].value() === references[1].value();
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(eq, args.capture());
}
41 changes: 41 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/gt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with >

Example:

```handlebars
{{gt age 17}}

{{! be true if `age` is > 17}}
```

@public
@method gt
@for Ember.Templates.helpers
@since 3.7.0
*/
function gt({ positional: { references } }: CapturedArguments) {
let left = references[0].value();
if (left === undefined || left === null) {
return false;
}
let right = references[1].value();
if (right === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this satisfy a specific edge case? I would have imagined that the following would have been enough:

return left > right;

Copy link
Contributor Author

@cibernox cibernox Nov 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried, but typescript doesn't like comparing things with null or undefined.

return false;
}
if (right === null) {
return left > 0;
}
return left > right;
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(gt, args.capture());
}
41 changes: 41 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/gte.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with >=

Example:

```handlebars
{{gt age 17}}

{{! be true if `age` is > 17}}
```

@public
@method gte
@for Ember.Templates.helpers
@since 3.7.0
*/
function gte({ positional: { references } }: CapturedArguments) {
let left = references[0].value();
if (left === undefined || left === null) {
return false;
}
let right = references[1].value();
if (right === undefined) {
return false;
}
if (right === null) {
return left >= 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd to me, why do we need to extra logic (vs just using the final return left >= right)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason. Typescript doesn't ket me compare things with undefined or null. How can we silence that?

}
return left >= right;
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(gte, args.capture());
}
41 changes: 41 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/lt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with <

Example:

```handlebars
{{lt age 17}}

{{! be true if `age` is < 17}}
```

@public
@method lt
@for Ember.Templates.helpers
@since 2.7.0
*/
function lt({ positional: { references } }: CapturedArguments) {
let left = references[0].value();
if (left === undefined || left === null) {
return false;
}
let right = references[1].value();
if (right === undefined) {
return false;
}
if (right === null) {
return left < 0;
}
return left < right;
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(lt, args.capture());
}
41 changes: 41 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/lte.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with <=

Example:

```handlebars
{{lte age 17}}

{{! be true if `age` is <= 17}}
```

@public
@method lte
@for Ember.Templates.helpers
@since 2.7.0
*/
function lte({ positional: { references } }: CapturedArguments) {
let left = references[0].value();
if (left === undefined || left === null) {
return false;
}
let right = references[1].value();
if (right === undefined) {
return false;
}
if (right === null) {
return left <= 0;
}
return left <= right;
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(lte, args.capture());
}
30 changes: 30 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/not-eq.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with !==

Example:

```handlebars
{{not-eq type "button"}}

{{! be true if `type` !== "button"}}
```

@public
@method not-eq
@for Ember.Templates.helpers
@since 3.7.0
*/
function notEq({ positional: { references } }: CapturedArguments) {
return references[0].value() !== references[1].value();
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(notEq, args.capture());
}
30 changes: 30 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/not.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Compares the two given values with ===

Example:

```handlebars
{{not disabled}}

{{! be true if `disabled` is false}}
```

@public
@method not
@for Ember.Templates.helpers
@since 3.7.0
*/
function not({ positional: { references } }: CapturedArguments) {
return !references[0].value();
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(not, args.capture());
}
37 changes: 37 additions & 0 deletions packages/@ember/-internals/glimmer/lib/helpers/or.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Arguments, CapturedArguments, VM } from '@glimmer/runtime';
import { InternalHelperReference } from '../utils/references';

/**
@module ember
*/

/**
Evaluates the given arguments with || in short-circuit

Example:

```handlebars
{{or isAdmin isAuthor}}

{{! be validData either `isAdmin` or `isAuthor` are truthy}}
```

@public
@method or
@for Ember.Templates.helpers
@since 3.7.0
*/
function or({ positional: { references } }: CapturedArguments) {
let last: any = false;
for (let i = 0; i < references.length; i++) {
last = references[i].value();
if (last) {
return last;
}
}
return last;
}

export default function(_vm: VM, args: Arguments) {
return new InternalHelperReference(or, args.capture());
}
22 changes: 22 additions & 0 deletions packages/@ember/-internals/glimmer/lib/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ENV } from '@ember/-internals/environment';
import { LookupOptions, Owner, setOwner } from '@ember/-internals/owner';
import { lookupComponent, lookupPartial, OwnedTemplateMeta } from '@ember/-internals/views';
import {
EMBER_BASIC_TEMPLATE_HELPERS,
EMBER_MODULE_UNIFICATION,
GLIMMER_CUSTOM_COMPONENT_MANAGER,
GLIMMER_MODIFIER_MANAGER,
Expand All @@ -29,14 +30,23 @@ import { default as htmlSafeHelper } from './helpers/-html-safe';
import { default as inputTypeHelper } from './helpers/-input-type';
import { default as normalizeClassHelper } from './helpers/-normalize-class';
import { default as action } from './helpers/action';
import { default as and } from './helpers/and';
import { default as array } from './helpers/array';
import { default as concat } from './helpers/concat';
import { default as eachIn } from './helpers/each-in';
import { default as eq } from './helpers/eq';
import { default as get } from './helpers/get';
import { default as gt } from './helpers/gt';
import { default as gte } from './helpers/gte';
import { default as hash } from './helpers/hash';
import { inlineIf, inlineUnless } from './helpers/if-unless';
import { default as log } from './helpers/log';
import { default as lt } from './helpers/lt';
import { default as lte } from './helpers/lte';
import { default as mut } from './helpers/mut';
import { default as not } from './helpers/not';
import { default as notEq } from './helpers/not-eq';
import { default as or } from './helpers/or';
import { default as queryParams } from './helpers/query-param';
import { default as readonly } from './helpers/readonly';
import { default as unbound } from './helpers/unbound';
Expand Down Expand Up @@ -84,6 +94,18 @@ const BUILTINS_HELPERS = {
'-outlet': outletHelper,
};

if (EMBER_BASIC_TEMPLATE_HELPERS) {
BUILTINS_HELPERS['and'] = and;
BUILTINS_HELPERS['eq'] = eq;
BUILTINS_HELPERS['gt'] = gt;
BUILTINS_HELPERS['gte'] = gte;
BUILTINS_HELPERS['lt'] = lt;
BUILTINS_HELPERS['lte'] = lte;
BUILTINS_HELPERS['not'] = not;
BUILTINS_HELPERS['not-eq'] = notEq;
BUILTINS_HELPERS['or'] = or;
}

if (DEBUG) {
BUILTINS_HELPERS['-assert-implicit-component-helper-argument'] = componentAssertionHelper;
}
Expand Down
Loading