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

Change how arrays/parens are handled #16

Open
techniq opened this issue Apr 6, 2018 · 0 comments
Open

Change how arrays/parens are handled #16

techniq opened this issue Apr 6, 2018 · 0 comments

Comments

@techniq
Copy link
Owner

techniq commented Apr 6, 2018

After some discussion in the PR about how to handle in and parens, I think we should implement a breaking change to make array ([ ]) usage more consistent and easier to reason about.

Current

Currently an array wraps each items within it in their own parens. (code)

} else if (Array.isArray(filters)) {
    const builtFilters = filters.map(f => buildFilter(f, propPrefix)).filter(f => f !== undefined);
    if (builtFilters.length) {
      return `${builtFilters.map(f => `(${f})`).join(` and `)}`
    }

which produces:

buildFilter({ filter: [{ SomeProp: 1 }, { AnotherProp: 2 }, "startswith(Name, 'R')"] });
=> "?$filter=(SomeProp eq 1) and (AnotherProp eq 2) and (startswith(Name, 'R'))";

Proposal

The proposal is to change the code to:

} else if (Array.isArray(filters)) {
    const builtFilters = filters.map(f => buildFilter(f, propPrefix)).filter(f => f !== undefined);
    if (builtFilters.length) {
      return `(${builtFilters.join(` and `)})`
    }

which would produce:

buildFilter({ filter: [{ SomeProp: 1 }, { AnotherProp: 2 }, "startswith(Name, 'R')"] });
=> "?$filter=(SomeProp eq 1 and AnotherProp eq 2 and startswith(Name, 'R'))";

You also do not need the individual objects for the first two (not a change, just a simplification of the example).

buildFilter({ filter: [{ SomeProp: 1, AnotherProp: 2 }, "startswith(Name, 'R')"] });
=> "?$filter=(SomeProp eq 1 and AnotherProp eq 2 and startswith(Name, 'R'))";

If you wanted to reproduce the original results (albeit with an extra wrapping parens around it all), you would wrap each item in an array:

buildFilter({ filter: [[{ SomeProp: 1 }], [{ AnotherProp: 2 }], ["startswith(Name, 'R')"]] });
=> "?$filter=((SomeProp eq 1) and (AnotherProp eq 2) and (startswith(Name, 'R')))";

In summary, the [ ] would literally translate as ( ) in the query. If this change is made, I would release it as a new major version since it could be breaking for some users.

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

1 participant