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

Fix Glint types by converting helpers to plain functions #188

Merged
merged 19 commits into from
Sep 2, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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: 3 additions & 2 deletions packages/ember-truth-helpers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"scripts": {
"build": "concurrently 'npm:build:*'",
"build:js": "rollup --config",
"build:types": "glint --declaration && grep -r -l '///' declarations/ | sort | uniq | xargs perl -e \"s/\\/\\/\\/.*$//\" -pi",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is fixed

"build:types": "glint --declaration",
"lint": "concurrently 'npm:lint:*(!fix)' --names 'lint:'",
"lint:fix": "concurrently 'npm:lint:*:fix' --names 'fix:'",
"lint:hbs": "ember-template-lint . --no-error-on-unmatched-pattern",
Expand All @@ -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
12 changes: 2 additions & 10 deletions packages/ember-truth-helpers/src/helpers/and.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
import { helper } from '@ember/component/helper';
import truthConvert from '../utils/truth-convert.ts';
import type { MaybeTruth } from '../utils/truth-convert.ts';

export interface AndSignature {
Args: {
Positional: MaybeTruth[];
};
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.

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

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
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
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
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
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
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
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
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
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
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
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
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
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