odata-query icon indicating copy to clipboard operation
odata-query copied to clipboard

Feature Request: Add option to forego Illegal Character Encoding

Open jstuder-gh opened this issue 5 years ago • 3 comments

Great library and very helpful in working with OData! However, there is one small pain point I have encountered in use.

In some instances, I'd like to perform HTML encoding on Unicode chars via encodeURIComponent. When doing that in conjunction with buildQuery, some of the chars get encoded twice.

Would you consider adding the option of passing in an argument to forego character escaping (defaulting to performing the escape of course, so as not to affect existing users).

Perhaps implemented as a second positional parameter that destructures into possible configuration options. Something like below for instance:

export default function({
  select,
  filter,
  search,
  groupBy,
  transform,
  orderBy,
  top,
  skip,
  key,
  count,
  expand,
  action,
  func,
  format
} = {},
{
  illegalCharEscape = true,
} = {}) {

An option like that would be very helpful for me, and I imagine other users as well.

jstuder-gh avatar May 24 '19 20:05 jstuder-gh

@jstuder-gh Thanks, I'm glad you're finding value with the proejct.

While not well documented, you can pass an object and specific the type as raw to handle the escape/etc.

See:

  • https://github.com/techniq/odata-query/issues/8#issuecomment-395499757
  • tests

I'm not against another approach if it makes since without complicating the API too much, but I think raw might serve your needs.

If we do go down another approach, I think maybe providing a way to override handleValue would provide for flexibility and cover more use cases (once again, if needed beyond raw).

Let me know what you think. PRs to improve docs/etc are always appreciated. Full disclosure, I've moved from OData to GraphQL for my current projects, but still try to maintain this project as best as I can.

techniq avatar May 25 '19 03:05 techniq

@techniq, thanks for the suggestion. That could definitely work, although since raw doesn't wrap the value in a single-quotes its not a perfect match for my use case (not that wrapping the string in quotes would be difficult to do on my side).

I do see how having it as a type helps in providing the functionality while being minimally invasive to the existing codebase. How would you feel about another type that foregoes the encoding like raw but also wraps the string in single-quotes? Like so:

https://github.com/jstuder-gh/odata-query/commit/53a76c20b7d833622f8b0637dceb5071079fc0c1

If you like this idea, I'd be happy to submit this (plus add a few related tests) as a PR. If not, then raw is perfectly workable. Thanks.

jstuder-gh avatar May 27 '19 18:05 jstuder-gh

I think I the use case is fine to support. If you send a PR with some tests I'll merge it and cut a release.

On Mon, May 27, 2019, 2:40 PM Jeremy Studer [email protected] wrote:

@techniq https://github.com/techniq, thanks for the suggestion. That could definitely work, although since raw doesn't wrap the value in a single-quotes its not a perfect match for my use case (not that wrapping the string in quotes would be difficult to do on my side).

I do see how having it as a type helps in providing the functionality while being minimally invasive to the existing codebase. How would you feel about another type that foregoes the encoding like raw but also wraps the string in single-quotes? Like so:

jstuder-gh@53a76c2 https://github.com/jstuder-gh/odata-query/commit/53a76c20b7d833622f8b0637dceb5071079fc0c1

If you like this idea, I'd be happy to submit this (plus add a few related tests) as a PR. If not, then raw is perfectly workable. Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/techniq/odata-query/issues/33?email_source=notifications&email_token=AABLKRE76GAFCWCA4N4H65TPXQTJXA5CNFSM4HPSLHDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWKK37Q#issuecomment-496283134, or mute the thread https://github.com/notifications/unsubscribe-auth/AABLKRDXBJROPEIJATK3RJDPXQTJXANCNFSM4HPSLHDA .

techniq avatar May 27 '19 20:05 techniq