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.

Sign up for GitHub

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

Fix Glint types by converting helpers to plain functions #188

Merged
merged 19 commits into from
Sep 2, 2023
Merged

Fix Glint types by converting helpers to plain functions #188

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
7f24e42
Copy contents from https://github.com/jmurphyau/ember-truth-helpers/p…
NullVoxPopuli Aug 28, 2023
ba8ac40
and
NullVoxPopuli Aug 28, 2023
5e30144
eq
NullVoxPopuli Aug 28, 2023
557ff7b
gt
NullVoxPopuli Aug 28, 2023
1c8ba28
gte
NullVoxPopuli Aug 28, 2023
0b89b30
is-array
NullVoxPopuli Aug 28, 2023
19d10cb
is-empty
NullVoxPopuli Aug 28, 2023
f0b46e7
is-equal
NullVoxPopuli Aug 28, 2023
290f72f
lt
NullVoxPopuli Aug 28, 2023
e6314cf
lte
NullVoxPopuli Aug 28, 2023
4b22c67
not-eq
NullVoxPopuli Aug 28, 2023
245e82a
not
NullVoxPopuli Aug 28, 2023
52e9a16
xor
NullVoxPopuli Aug 28, 2023
9f7347f
Add polyfill to support 3.28+
NullVoxPopuli Aug 28, 2023
ce51051
andfix
NullVoxPopuli Aug 29, 2023
b21676d
We no longer need sed to fix /// referenecs
NullVoxPopuli Aug 29, 2023
b8e90e2
make `and` and `or` helpers class based
Aug 31, 2023
7b251ea
add short-circuit tests for `and` and `or` helpers
Sep 2, 2023
9766373
add Glint check for inferred type of `and` and `or` helpers
Sep 2, 2023
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
3 changes: 2 additions & 1 deletion packages/ember-truth-helpers/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"prepack": "rollup --config"
},
"dependencies": {
"@embroider/addon-shim": "^1.8.6"
"@embroider/addon-shim": "^1.8.6",
"ember-functions-as-helper-polyfill": "^2.1.2"
},
"devDependencies": {
"@babel/core": "^7.22.9",
Expand Down
5 changes: 2 additions & 3 deletions packages/ember-truth-helpers/src/helpers/and.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { helper } from '@ember/component/helper';
import truthConvert from '../utils/truth-convert.ts';
import type { MaybeTruth } from '../utils/truth-convert.ts';

Expand All @@ -9,11 +8,11 @@ export interface AndSignature {
Return: boolean;
}

export default helper<AndSignature>((params) => {
export default function and<T extends MaybeTruth[]>(...params: [...T]) {
NullVoxPopuli marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

@NullVoxPopuli using spread causes an issue - today, arguments gets lazily evaluated and spread makes it process eager.

e.g. imagine use case

{{and (foo "A") (bar "B")}}

today, if (foo "A") returns false, then (bar "B") does not get evaluated.
but this line change makes it evaluated which is definitely not what we want

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's just how plain functions work though: https://github.com/glimmerjs/glimmer-vm/blob/68d371bdccb41bc239b8f70d832e956ce6c349d8/packages/%40glimmer/manager/lib/internal/defaults.ts#L38

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to make or lazy or short-circuit, it needs to be built in to the VM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if or and and need to remain "classic" I wonder if that means converting to class helpers so that we can correctly use generics?

for (let i = 0, len = params.length; i < len; i++) {
if (truthConvert(params[i]) === false) {
return params[i] as boolean;
}
}
return params[params.length - 1] as boolean;
Copy link
Contributor

@Techn1x Techn1x Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

keeping the as boolean's here might not quite meet the type needs of #185 for the and helper

Copy link
Contributor

Choose a reason for hiding this comment

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

(not that this PR needs to solve that too)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I'd prefer to add tests for the scenarios in #185 -- but in a separate PR -- since this PR is mostly a mechanical change branched off of #185, I'd prefer to iterate, so that PRs can remain reviewable

});
}
13 changes: 2 additions & 11 deletions packages/ember-truth-helpers/src/helpers/eq.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
import { helper } from '@ember/component/helper';

export interface EqSignature {
Args: {
Positional: [unknown, unknown];
};
Return: boolean;
export default function eq(left: unknown, right: unknown): boolean {
return left === right;
}

export default helper<EqSignature>((params) => {
return params[0] === params[1];
});
22 changes: 7 additions & 15 deletions packages/ember-truth-helpers/src/helpers/gt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { helper } from '@ember/component/helper';

export interface GtSignature {
Args: {
Positional: [unknown, unknown];
Named: {
forceNumber?: boolean;
};
};
Return: boolean;
}

export default helper<GtSignature>(([left, right], options) => {
if (options.forceNumber) {
export default function gt(
left: unknown,
right: unknown,
options?: { forceNumber?: boolean }
) {
if (options?.forceNumber) {
if (typeof left !== 'number') {
left = Number(left);
}
Expand All @@ -20,4 +12,4 @@ export default helper<GtSignature>(([left, right], options) => {
}
}
return (left as number) > (right as number);
});
}
22 changes: 7 additions & 15 deletions packages/ember-truth-helpers/src/helpers/gte.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { helper } from '@ember/component/helper';

export interface GteSignature {
Args: {
Positional: [unknown, unknown];
Named: {
forceNumber?: boolean;
};
};
Return: boolean;
}

export default helper<GteSignature>(([left, right], options) => {
if (options.forceNumber) {
export default function gte(
left: unknown,
right: unknown,
options?: { forceNumber?: boolean }
) {
if (options?.forceNumber) {
if (typeof left !== 'number') {
left = Number(left);
}
Expand All @@ -20,4 +12,4 @@ export default helper<GteSignature>(([left, right], options) => {
}
}
return (left as number) >= (right as number);
});
}
15 changes: 3 additions & 12 deletions packages/ember-truth-helpers/src/helpers/is-array.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
import { helper } from '@ember/component/helper';
import { isArray } from '@ember/array';
import type EmberArray from '@ember/array';
import { isArray as isEmberArray } from '@ember/array';

export interface IsArraySignature {
Args: {
Positional: unknown[] | EmberArray<unknown>;
};
Return: boolean;
export default function isArray(...params: unknown[]) {
return params.every(isEmberArray);
}

export default helper<IsArraySignature>((params) => {
return params.every(isArray);
});
12 changes: 1 addition & 11 deletions packages/ember-truth-helpers/src/helpers/is-empty.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
import { helper } from '@ember/component/helper';
import { isEmpty } from '@ember/utils';

export interface IsEmptySignature {
Args: {
Positional: [unknown];
};
Return: boolean;
}

export default helper<IsEmptySignature>(([obj]) => {
return isEmpty(obj);
});
export default isEmpty;
12 changes: 1 addition & 11 deletions packages/ember-truth-helpers/src/helpers/is-equal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
import { helper } from '@ember/component/helper';
import { isEqual } from '@ember/utils';

export interface IsEqualSignature {
Args: {
Positional: [unknown, unknown];
};
Return: boolean;
}

export default helper<IsEqualSignature>(([a, b]) => {
return isEqual(a, b);
});
export default isEqual;
22 changes: 7 additions & 15 deletions packages/ember-truth-helpers/src/helpers/lt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { helper } from '@ember/component/helper';

export interface LtSignature {
Args: {
Positional: [unknown, unknown];
Named: {
forceNumber?: boolean;
};
};
Return: boolean;
}

export default helper<LtSignature>(([left, right], options) => {
if (options.forceNumber) {
export default function lt(
left: unknown,
right: unknown,
options?: { forceNumber?: boolean }
) {
if (options?.forceNumber) {
if (typeof left !== 'number') {
left = Number(left);
}
Expand All @@ -20,4 +12,4 @@ export default helper<LtSignature>(([left, right], options) => {
}
}
return (left as number) < (right as number);
});
}
22 changes: 7 additions & 15 deletions packages/ember-truth-helpers/src/helpers/lte.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { helper } from '@ember/component/helper';

export interface LteSignature {
Args: {
Positional: [unknown, unknown];
Named: {
forceNumber?: boolean;
};
};
Return: boolean;
}

export default helper<LteSignature>(([left, right], options) => {
if (options.forceNumber) {
export default function lte(
left: unknown,
right: unknown,
options?: { forceNumber?: boolean }
) {
if (options?.forceNumber) {
if (typeof left !== 'number') {
left = Number(left);
}
Expand All @@ -20,4 +12,4 @@ export default helper<LteSignature>(([left, right], options) => {
}
}
return (left as number) <= (right as number);
});
}
13 changes: 2 additions & 11 deletions packages/ember-truth-helpers/src/helpers/not-eq.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
import { helper } from '@ember/component/helper';

export interface NotEqSignature {
Args: {
Positional: [unknown, unknown];
};
Return: boolean;
export default function notEq(left: unknown, right: unknown) {
return left !== right;
}

export default helper<NotEqSignature>((params) => {
return params[0] !== params[1];
});
12 changes: 2 additions & 10 deletions packages/ember-truth-helpers/src/helpers/not.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
import { helper } from '@ember/component/helper';
import truthConvert from '../utils/truth-convert.ts';
import type { MaybeTruth } from '../utils/truth-convert.ts';

export interface NotSignature {
Args: {
Positional: MaybeTruth[];
};
Return: boolean;
}

export default helper<NotSignature>((params) => {
export default function not(...params: MaybeTruth[]) {
return params.every((param) => !truthConvert(param));
});
}
34 changes: 22 additions & 12 deletions packages/ember-truth-helpers/src/helpers/or.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
import { helper } from '@ember/component/helper';
import truthConvert from '../utils/truth-convert.ts';
import type { MaybeTruth } from '../utils/truth-convert.ts';

export interface OrSignature {
Args: {
Positional: MaybeTruth[];
};
Return: boolean;
}

export default helper<OrSignature>((params) => {
export default function or<T extends MaybeTruth[]>(...params: [...T]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same issue here as in and helper

for (let i = 0, len = params.length; i < len; i++) {
if (truthConvert(params[i]) === true) {
return params[i] as boolean;
return params[i] as FirstNonFalsy<T>;
}
}
return params[params.length - 1] as boolean;
});
return params[params.length - 1] as Last<T>;
}

type FirstNonFalsy<T extends any[]> = T extends [infer K]
? K
: T extends [infer A, ...infer R]
? Truthy<A> extends true
? A
: FirstNonFalsy<R>
: never;

type Last<T extends any[]> = T extends [infer K]
? K
: T extends [...infer Q, infer L]
? L
: never;

type Truthy<T> = T extends Falsy ? false : true;

type Falsy = false | 0 | '' | null | undefined | { isTruthy: false } | [];
7 changes: 3 additions & 4 deletions packages/ember-truth-helpers/src/helpers/xor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { helper } from '@ember/component/helper';
import truthConvert from '../utils/truth-convert.ts';

export interface XorSignature {
Expand All @@ -8,6 +7,6 @@ export interface XorSignature {
Return: boolean;
}

export default helper<XorSignature>((params) => {
return truthConvert(params[0]) !== truthConvert(params[1]);
});
export default function xor(left: unknown, right: unknown) {
return truthConvert(left) !== truthConvert(right);
}
Loading