Precedence conflict between assignment and object plugins

zcappp commented 3 years ago

among those new plugins I thirst for object plugin most, thank you to make it possible! when I register both assignment and object plugins, it make combination of assignment and ternary error out. such as a ? log(a) : a = 1 we expect to compute test a first and then the assignment later on:

{
    "type": "ConditionalExpression",
    "test": {
        "type": "Identifier",
        "name": "a"
    },
    "consequent": {
        "type": "CallExpression",
        "arguments": [
            {
                "type": "Identifier",
                "name": "a"
            }
        ],
        "callee": {
            "type": "Identifier",
            "name": "log"
        }
    },
    "alternate": {
        "type": "AssignmentExpression",
        "operator": "=",
        "left": {
            "type": "Identifier",
            "name": "a"
        },
        "right": {
            "type": "Literal",
            "value": 1,
            "raw": "1"
        }
    }
}

but in fact it will throw out error:

evaluate.js:66 Error: Expected : at character 17
    at Jsep.throwError (jsep.js:327)
    at Jsep.gobbleTernary (jsep.js:1126)
    at jsep.js:53
    at Array.forEach (<anonymous>)
    at Hooks.run (jsep.js:52)
    at Jsep.runHook (jsep.js:342)
    at Jsep.gobbleExpression (jsep.js:440)
    at Jsep.gobbleExpressions (jsep.js:415)
    at Jsep.parse (jsep.js:385)
    at jsep (jsep.js:1040)

but if I group the assignment with parentheses： a ? log(a) : (a = 1) jsep can parse as expected.

I remove the registration of assignment plugin, and add it as binaryOP: jsep.addBinaryOp("=", 0.96) then jsep works as expected!

note: I give it precedence of 0.96 as I found assignment plugin have jsep.addBinaryOp(':', 0.95)

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 1.1.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version @jsep-plugin/assignment-v1.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version @jsep-plugin/ternary-v1.0.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

zcappp commented 3 years ago

@6utt3rfly Thank you for the quick fixing! as the ternary plugin is built-in by default, I upgraded jsep core and assignment plugin to latest, ignoring ternary, it seems error still there. can you check if latest ternary is included in jsep core package?

by the way, when excecuting "yarn upgrade", there are warnings: Workspaces can only be enabled in private projects I guess it was caused by workspaces statement in package.json

6utt3rfly commented 3 years ago

@zclc - it looks like there are still a couple issues remaining, sorry. I've added issue #195 to figure out how we want to deal with left/right-associativity, which is ultimately the root cause of this issue. In the meantime, parentheses will help you work around the issue until we can implement a more complete solution.

Also, thanks for the note on the workspaces warning too - that shouldn't be hard to fix 🙂

6utt3rfly commented 3 years ago

@zclc - would you like to try manually testing the PR to see if it resolves your issues?

zcappp commented 3 years ago

@6utt3rfly I just reinstalled latest jsep and its plugins, the issue is still there. I am wondering if the latest packages are really built from latest source code. I saw there is v1.2.0 release, but the version on package.json is still on 1.0.2. below are all the lastest versions I got:

"@jsep-plugin/assignment": "^1.0.1",
"@jsep-plugin/comment": "^1.0.1",
"@jsep-plugin/new": "^1.0.1",
"@jsep-plugin/object": "^1.1.0",
"@jsep-plugin/regex": "^1.0.1",
"jsep": "^1.1.0",

6utt3rfly commented 3 years ago

@zclc We haven't built a new release with PR #199 (still in review). Are you able to manually use the source from that PR to try?

zcappp commented 3 years ago

I tried, but I didn't get to know how to pull PR yet. by the way I also failed to build jsep locally. currenly I am using acorn instead to parse my expression, but I still like the lightweight of jsep.