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

Incorrect types in jsonata.d.ts #585

Open
socjalis opened this issue Jul 6, 2022 · 4 comments
Open

Incorrect types in jsonata.d.ts #585

socjalis opened this issue Jul 6, 2022 · 4 comments

Comments

@socjalis
Copy link

socjalis commented Jul 6, 2022

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

Please update the types.

@mattbaileyuk
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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

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

No branches or pull requests

4 participants