jsonata-js / jsonata

JSONata query and transformation language - http://jsonata.org
MIT License
2.06k stars 220 forks source link

Incorrect types in jsonata.d.ts #585

Open socjalis opened 2 years ago

socjalis commented 2 years ago

According to the included types lhs in ExprNode should be of type ExprNode itself. Meanwhile:

const JSONATA_STRING = `{"x": y}`;
const expression = jsonata(JSONATA_STRING);
console.log(JSON.stringify(expression.ast()));

resolves to:

{
  "type":"unary",
  "value":"{",
  "position":1,
  "lhs":[
    [
      {
        "value":"x",
        "type":"string",
        "position":4
      },
      {
        "type":"path",
        "steps":[
          {
            "value":"y",
            "type":"name",
            "position":7
          }
        ]
      }
    ]
  ]
}

As you can see, lhs is an array here. Incorrect types make it really hard to traverse the whole expression.ast() tree without checking 2k+ lines source code files, which makes the developer experience not that fun.

Also I didn't really go that deep into the source code, but something like this seems more intuitive at the first glance and is coherent with the provided types:

{
  "type":"unary",
  "value":"{",
  "position":1,
  "steps?/stages?":[
    {
      "type":"binary",
      "value":":",
      "position":3,
      "lhs":{
        "value":"x",
        "type":"string",
        "position":4
      },
      "rhs":{
        "type":"path",
        "steps":[
          {
            "value":"y",
            "type":"name",
            "position":7
          }
        ]
      }
    }
  ]
}
koladilip commented 1 year ago

Please update the types.

mattbaileyuk commented 1 year ago

Code contributions are welcome here 🙂 The jsonata.d.ts TypeScript file was an outside contribution, and I don't have much familiarity with it - not sure if @andrew-coleman does either. I'd like to get around to looking at it at some point soon, but this would certainly get addressed quicker if anyone who does understand can provide a PR.

dkaushik95 commented 1 year ago

Hi, @mattbaileyuk, I can help with this. Before I make changes, do we know that lhs returns ExprNode array always or are there exceptions? @socjalis can you confirm? Thanks!

mattbaileyuk commented 1 year ago

@dkaushik95 Thanks, and yes, I believe it should always be an array, including an empty one.