hapijs / joi

The most powerful data validation library for JS
Other
20.95k stars 1.51k forks source link

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

Closed hobgoblina closed 1 year ago

hobgoblina commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I got this PR up on the formula repo that solves the issue in the above comment: https://github.com/hapijs/formula/pull/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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/hapijs/joi/pull/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 commented 1 year ago

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.

hobgoblina commented 1 year ago

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

Marsup commented 1 year ago

It's already released in 17.10.2

hobgoblina commented 1 year ago

confirmed that it's working now, thanks!!