⚡ use qs_dart for query string encoding

[techouse]

@diegotori noticed that a Map<String, dynamic> like this

{
  'filters': {
    r'$or': [
      {
        'date': {
          r'$eq': '2020-01-01',
        }
      },
      {
        'date': {
          r'$eq': '2020-01-02',
        }
      }
    ],
    'author': {
      'name': {
        r'$eq': 'Kai doe',
      },
    }
  }
}

did not get encoded properly to

filters[$or][][date][$eq]=2020-01-01&filters[$or][][date][$eq]=2020-01-02&filters[author][name][$eq]=Kai%20doe

but became this

filters[$or][]={date: {$eq: 2020-01-01}}&filters[$or][]={date: {$eq: 2020-01-02}}&filters[author][name][$eq]=Kai doe

This PR fixes this by using an external library qs_dart which is a Dart port of qs that I've been painstakingly working on over the past couple of weeks. 🤓

This now means that the output of Chopper, while still backwards compatible, is now qs compatible.

The only caveat is null and empty String handling in query strings, which is a bit different in qs and therefore also qs_dart.

Please check the various test case changes to see these null and empty String differences.

---

Addresses #584
Supersedes #591

CC / @JEuler @Guldem @diegotori

[codecov]

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 93.52%. Comparing base (dd61ec1) to head (34d1d06).

❗ Current head 34d1d06 differs from pull request most recent head 7a6c67a. Consider uploading reports for the commit 7a6c67a to get more accurate results

Files	Patch %	Lines
chopper/lib/src/list_format.dart	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #592      +/-   ##
===========================================
- Coverage    94.19%   93.52%   -0.68%     
===========================================
  Files           11       12       +1     
  Lines          482      463      -19     
===========================================
- Hits           454      433      -21     
- Misses          28       30       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[techouse]

I propose this goes into v8.0.0 since it slightly changes the way null and empty Strings are encoded.

CC / @JEuler @Guldem @diegotori

[Guldem]

I propose this goes into v8.0.0 since it slightly changes the way null and empty Strings are encoded.

CC / @JEuler @Guldem @diegotori

Makes sense. Introduce all changes at once 😉

LGTM! Also want someone to have a look at qs_dart?

[techouse]

LGTM! Also want someone to have a look at qs_dart?

Please :) I converted all the tests of qs, however, porting the JavaScript code was quite the ordeal since it's not typed and written in such a legacy way that made me cry a few times 🙈

[Guldem]

LGTM! Also want someone to have a look at qs_dart?

Please :) I converted all the tests of qs, however, porting the JavaScript code was quite the ordeal since it's not typed and written in such a legacy way that made me cry a few times 🙈

Have looked globally at the code of qs and qs_dart and the tests which seemed alright. Since the test cases cover a lot I'm sure its working or needs small fixes. Great job porting it! ❤️

[diegotori]

@techouse I still need to add the encoding tests that evaluate Strapi query map encoding into this library's tests.

Also, please consider adding an option for indexed bracketed arrays, unless qs_dart already does that by default, then we're already good.

[techouse]

Also, please consider adding an option for indexed bracketed arrays, unless qs_dart already does that by default, then we're already good.

qs_dart indeed defaults to ListFormat.indices, however, Chopper does not.

I'm working on deprecating the current tags (while still keeping them) and adding a new ones which will give the user an option to encode with any of these:

• brackets (current default)
• repeat
• indices
• comma

[diegotori]

@techouse I think we should also target this for 7.4.0, so that it's available immediately.

Otherwise, the changes related to the interceptors might get in the way if we have to upgrade to 8.0.0.

Not to mention, it gives you time to deprecate useBrackets and replace it with ListFormat.

[Guldem]

@techouse I think we should also target this for 7.4.0, so that it's available immediately.
Otherwise, the changes related to the interceptors might get in the way if we have to upgrade to 8.0.0.
Not to mention, it gives you time to deprecate useBrackets and replace it with ListFormat.

Hmm, fair point.

@Guldem are you onboard with this?

That's fine! This is needed, the interceptors not necessarily 😄

[techouse]

I've added a new annotation listFormat

/// List format to use when encoding lists
///
/// - [ListFormat.repeat] (default) hxxp://path/to/script?foo=123&foo=456&foo=789
/// - [ListFormat.brackets] hxxp://path/to/script?foo[]=123&foo[]=456&foo[]=789
/// - [ListFormat.indices] hxxp://path/to/script?foo[0]=123&foo[1]=456&foo[2]=789
/// - [ListFormat.comma] hxxp://path/to/script?foo=123,456,789
final ListFormat? listFormat;

and have deprecated useBrackets

/// Use brackets [ ] to when encoding
///
/// - lists
/// hxxp://path/to/script?foo[]=123&foo[]=456&foo[]=789
///
/// - maps
/// hxxp://path/to/script?user[name]=john&user[surname]=doe&user[age]=21
@Deprecated('Use listFormat instead')
final bool? useBrackets;

In line with the above I have also modified mapToQuery

String mapToQuery(
  Map<String, dynamic> map, {
  ListFormat? listFormat,
  @Deprecated('Use listFormat instead') bool? useBrackets,
  bool? includeNullQueryVars,
}) {
  listFormat ??= useBrackets == true ? ListFormat.brackets : ListFormat.repeat;

  return QS.encode(
    map,
    EncodeOptions(
      listFormat: listFormat,
      allowDots: listFormat == ListFormat.repeat,
      encodeDotInKeys: listFormat == ListFormat.repeat,
      encodeValuesOnly: listFormat == ListFormat.repeat,
      skipNulls: includeNullQueryVars != true,
      strictNullHandling: false,
      serializeDate: (DateTime date) => date.toUtc().toIso8601String(),
    ),
  );
}

and added a tonne of tests 🤪

CC / @JEuler @Guldem @diegotori

[techouse]

@diegotori could you please test this PR in your problematic project/app and report back any findings?

dependency_overrides:
  chopper:
    git:
      url: https://github.com/lejard-h/chopper.git
      ref: fix/issue-584-qs
      path: chopper
  chopper_generator:
    git:
      url: https://github.com/lejard-h/chopper.git
      ref: fix/issue-584-qs
      path: chopper_generator

[diegotori]

dependency_overrides:
  chopper:
    git:
      url: https://github.com/lejard-h/chopper.git
      ref: fix/issue-584-qs
      path: chopper
  chopper_generator:
    git:
      url: https://github.com/lejard-h/chopper.git
      ref: fix/issue-584-qs
      path: chopper_generator

I dunno if I have the capacity to do that currently. However, once you release it and we start the process of migrating to using this new version, if there are any issues, we'll be sure to raise them.

So far, the tests that were added to both this library and qs_dart are reflective of the issues that we ran into when trying to create query strings from maps in chopper.

Since qs_dart is literally a 1:1 port of JavaScript's qs library (which Strapi uses), then I'm highly confident that this new version will work flawlessly.

[techouse]

Since qs_dart is literally a 1:1 port of JavaScript's qs library (which Strapi uses), then I'm highly confident that this new version will work flawlessly.

I tried my best to port everything that could be ported, however, I could not make it a 1:1 port because of some core differences between JavaScript and Dart, i.e. Object prototypes, sparse arrays, etc.

[diegotori]

Since qs_dart is literally a 1:1 port of JavaScript's qs library (which Strapi uses), then I'm highly confident that this new version will work flawlessly.

I tried my best to port everything that could be ported, however, I could not make it a 1:1 port because of some core differences between JavaScript and Dart, i.e. Object prototypes, sparse arrays, etc.

No worries. So far, it's spitting out Strapi-compatible query strings, and that's what really matters for us.

That way, we won't have to do backend hackery to get around the previous limitations.

Standby for another set of tests that you'll need to add so that it covers ones that I found.

[diegotori]

LGTM.

[techouse]

I identified another bug in qs_dart. I forgot to serialize any object by calling .toString() on it.

EDIT: Fixed