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

[Feat]: logical keywords #2

Closed
wants to merge 0 commits into from

Conversation

snewcomer
Copy link
Owner

@snewcomer snewcomer commented Mar 3, 2021


function isTruthy(result: unknown) {
if (Array.isArray(result)) {
return result.length !== 0;
Copy link

Choose a reason for hiding this comment

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

I have some concerns about this. If this isn't a done deal I'd like to raise them. Where should I do that?

Copy link

Choose a reason for hiding this comment

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

I guess for now I can just say them here. Suppose someone had the following:

{{or someMaybeEmptyArray otherArray}}

If we're trying to do TS types for this we can't just convert to someMaybeEmptyArray || otherArray. As long as someMaybeEmptyArray isn't null or undefined TS will see it as being truthy and will just return the type as someMaybeEmptyArray however, the correct type would actually be someMaybeEmptyArray | otherArray. In the case where someone wants to handle empty arrays, I think it would more correctly done as:

{{if someMaybeEmptyArray.length someMaybeEmptyArray otherArray}}

We could then correctly type that as someMaybeEmptyArray | otherArray.

snewcomer pushed a commit that referenced this pull request Apr 13, 2021
Updating to @handlebars/parser@2.1.0 introduces a number of test
failures that need to be debugged:

```js
not ok 1378 Chrome 89.0 - [2 ms] - [integration] jit :: {{log}} keyword: correctly logs `this`
    ---
        actual: >
            null
        stack: >
            TypeError: Cannot read property 'length' of undefined
                at ExpressionNormalizer.path (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38770:23)
                at ExpressionNormalizer.normalize (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38746:23)
                at http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38795:22
                at Array.map (<anonymous>)
                at ExpressionNormalizer.callParts (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38794:30)
                at StatementNormalizer.MustacheStatement (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38956:33)
                at StatementNormalizer.normalize (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38900:23)
                at http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38625:25
                at Array.map (<anonymous>)
                at Object.normalize (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38624:67)
        message: >
            Died on test #2     at _loop (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17888:21)
                at http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17898:11
                at suite (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17900:9)
                at jitSuite (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17814:12)
                at Module.callback (http://localhost:7357/153311786233/tests/assets/tests.js:9760:18)
                at Module.exports (http://localhost:7357/153311786233/tests/assets/loader.js:106:32)
                at requireModule (http://localhost:7357/153311786233/tests/assets/loader.js:27:18)
                at http://localhost:7357/153311786233/tests/index.html?hidepassed:140:11: Cannot read property 'length' of undefined
        negative: >
            false
        browser log: |

not ok 1415 Chrome 89.0 - [3 ms] - [integration] jit :: modifiers: modifiers on components accept `this` in both positional params and named arguments, and updates when it changes
    ---
        actual: >
            null
        stack: >
            TypeError: Cannot read property 'length' of undefined
                at ExpressionNormalizer.path (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38770:23)
                at ExpressionNormalizer.normalize (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38746:23)
                at http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38795:22
                at Array.map (<anonymous>)
                at ExpressionNormalizer.callParts (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38794:30)
                at ElementNormalizer.modifier (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:39108:33)
                at http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:39059:23
                at Array.map (<anonymous>)
                at ElementNormalizer.ElementNode (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:39058:41)
                at StatementNormalizer.normalize (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:38897:52)
        message: >
            Died on test #2     at _loop (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17888:21)
                at http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17898:11
                at suite (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17900:9)
                at jitSuite (http://localhost:7357/153311786233/tests/assets/glimmer-vm.js:17814:12)
                at Module.callback (http://localhost:7357/153311786233/tests/assets/tests.js:11623:18)
                at Module.exports (http://localhost:7357/153311786233/tests/assets/loader.js:106:32)
                at requireModule (http://localhost:7357/153311786233/tests/assets/loader.js:27:18)
                at http://localhost:7357/153311786233/tests/index.html?hidepassed:140:11: Cannot read property 'length' of undefined
        negative: >
            false
        browser log: |
    ...
```
@snewcomer snewcomer force-pushed the sn/logical-operators branch 2 times, most recently from e97a9c7 to 3e45f7e Compare April 13, 2021 03:52
@snewcomer snewcomer force-pushed the sn/comparison-operators branch from caa3895 to 155e30b Compare April 13, 2021 03:54
@snewcomer snewcomer force-pushed the sn/logical-operators branch from 3e45f7e to a40dc36 Compare April 13, 2021 03:58
@snewcomer snewcomer closed this Apr 13, 2021
@snewcomer snewcomer force-pushed the sn/logical-operators branch from a40dc36 to 678580a Compare April 13, 2021 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants