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

Unable to access object property with a space in its key via expression templates #2974

Closed
hobgoblina opened this issue Aug 14, 2023 · 9 comments · Fixed by #2986
Closed
Assignees
Labels
bug Bug or defect support Questions, discussions, and general support
Milestone

Comments

@hobgoblina
Copy link

hobgoblina commented Aug 14, 2023

Hi, I'm attempting to access a property on an object that contains a space in the property's key.

I've got a value that could be structured as any of the following:

value: {
  ratio: 'actual value I need to access'
  // other properties
}
value: {
  'frequency ratio': 'actual value I need to access'
  // other properties
}
value: 'actual value I need to access'

And I'm trying to use this syntax to capture all possible structures (my actual error case is array.unique, but that's beside the point so I reduced this to a single value rather than value + dupeValue):
Invalid frequency ratio: "{#value.ratio || #value["frequency ratio"] || #value}"

The dot notation accessor works fine, but the bracket notation accessor results in this error:
TypeError: Cannot read properties of undefined (reading 'Symbol(formula)')

Here's the relevant stack trace:

      at Object.<anonymous>.internals.evaluate (node_modules/@sideway/formula/lib/index.js:385:13)
      at node_modules/@sideway/formula/lib/index.js:347:44
          at Array.forEach (<anonymous>)
      at Object.<anonymous>.exports.Parser.evaluate (node_modules/@sideway/formula/lib/index.js:342:34)
      at Object.<anonymous>.module.exports.internals.Template._part (node_modules/joi/lib/template.js:178:29)
      at Object.<anonymous>.module.exports.internals.Template.render (node_modules/joi/lib/template.js:193:39)

I see that brackets are parsed out in the formula library due to being treated as literals, so I've tried various other things to get past this, but nothing that I've tried is working. Is this just a current limitation, or is there a workaround that I'm not finding?

Thanks!

@hobgoblina hobgoblina added the support Questions, discussions, and general support label Aug 14, 2023
@hobgoblina
Copy link
Author

My usual debugging strategy works yet again:

  1. Try until I'm too frustrated
  2. Ask for help
  3. Figure it out on my own within minutes after asking for help

For anyone who runs into this problem, the correct syntax is #value.[property key with spaces]

@hobgoblina
Copy link
Author

Never mind... I'm now getting the same error when the value isn't an object like in the 3rd example above...

value: 'actual value I need to access'

Trying to find a workaround for this now.

@hobgoblina
Copy link
Author

I got this PR up on the formula repo that solves the issue in the above comment: hapijs/formula#14

As I mentioned in the PR, I'm not sure if there's a deeper issue that needs to be resolved here, but this fixes my issue and it seems like a sensible change to me.

@Marsup
Copy link
Collaborator

Marsup commented Sep 13, 2023

Hi, and sorry for the delay.

Could you maybe submit a failing unit test? Because I'm having troubles reproducing the issue on my side.

From what I'm seeing, it could be a misunderstanding of the template syntax, which is not exactly javascript. I think your formula should be written like this: "{#value.ratio || #value.[frequency ratio] || #value}". As mentioned in the docs, square brackets are there to disambiguate references, it's not an accessor.

I'm still interested in understanding the reasons for this stacktrace though, so I'll hold your PR until I can see how to debug it myself.

@hobgoblina
Copy link
Author

hobgoblina commented Sep 13, 2023

Yeah, that's the formula that I'm using now (is the syntax I was referring to in the first reply I made here).

Here's a PR with unit tests showing the cases where the bug is occurring: #2985 (probably could've used a simpler schema without the unique, but I just modified the schema I'm using since it's what's failing for me)

As you can see, that syntax is throwing the error anytime #value isn't an object.

I didn't include examples to show this part: this bug seems to only be happening when the formula includes the object property with spaces (ie, #value.[frequency ratio]... or #value.[var with spaces] in the unit tests). If I make the formula "{#value.ratio || #value}" then I don't have any issues (aside from it not covering all possible cases 😛).

Marsup added a commit that referenced this issue Sep 17, 2023
@Marsup Marsup linked a pull request Sep 17, 2023 that will close this issue
@Marsup Marsup self-assigned this Sep 17, 2023
@Marsup Marsup added the bug Bug or defect label Sep 17, 2023
@Marsup Marsup added this to the 17.10.2 milestone Sep 17, 2023
@Marsup
Copy link
Collaborator

Marsup commented Sep 17, 2023

Thanks a lot for your patience and for your efforts in trying to fix this. After digging, I decided to change joi's resolve function instead of changing formula's. Hopefully I'm not breaking anyone's code while still fixing the issue. I'm pretty sure it will fix your use case, but do let know if not.

@Marsup Marsup closed this as completed Sep 17, 2023
@hobgoblina
Copy link
Author

sounds good, thanks! is this already in a release or expected for an upcoming one?

@Marsup
Copy link
Collaborator

Marsup commented Sep 17, 2023

It's already released in 17.10.2

@hobgoblina
Copy link
Author

confirmed that it's working now, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug or defect support Questions, discussions, and general support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants