os-js / osjs-client

OS.js Client Module
https://manual.os-js.org/
Other
31 stars 31 forks source link

Improve serialization of VFS URL query parameters #165

Closed mahsashadi closed 2 years ago

mahsashadi commented 2 years ago

The implementation of encodeQueryData method changes to support encoding of nested objects.

Relevant:

mahsashadi commented 2 years ago

As you can see from this test and the one in osjs-server, it demonstrates that issue with all values being strings

Yes, You are totally right. We should preserve value type somehow.

mahsashadi commented 2 years ago

To support encoding arrays, we can change method as below :thinking:

function encodeQueryData(data, nesting = '') {
  const pairs = Object.entries(data).map(([key, val]) => {
    if (typeof val === 'object' && val !== null) {
      return Array.isArray(val)
        ? encodeQueryData(val, nesting + `${key}`)
        : encodeQueryData(val, nesting + `${key}.`);
    } else {
      return Array.isArray(data)
        ? encodeURIComponent(nesting) + '=' + encodeURIComponent(val)
        : encodeURIComponent(nesting + key) + '=' + encodeURIComponent(val);
    }
  });
  return pairs.join('&');
}

Do you have a better idea?

mahsashadi commented 2 years ago

We should preserve value type somehow.

I have not yet reached a solution to this problem :thinking:

andersevenrud commented 2 years ago

Technically the serialization is correct, so I don't really think this needs to be changed to support arrays.

mahsashadi commented 2 years ago

Thanks a lot for your help :) Sorry if it takes your time. I learned a lot during this PR 👍

andersevenrud commented 2 years ago

Thanks a lot for your help :)

And thank you for making these changes.

Sorry if it takes your time.

Don't worry about that. I haven't spent any time I don't have to look at this.

I learned a lot during this PR

Glad to hear that :)

mahsashadi commented 2 years ago

Hi again :) To continue work on filemanager pagination, I have commited some changes:

mahsashadi commented 2 years ago

Hi again, did you find anytime to take a look at my new commited codes?

andersevenrud commented 2 years ago

Sorry. Personal stuff has prevented me from doing much lately. Sorry about that, but I will get to this ASAP.

andersevenrud commented 2 years ago

Could you split up this PR so that it only contains the URL changes ?

Right now this technically contains two different kinds of work, and I don't want to merge everything at once.

mahsashadi commented 2 years ago

@andersevenrud Here is the subset of changes for URL serialization:

https://github.com/os-js/osjs-client/pull/182

Here is the subset of changes for URL unserialization:

https://github.com/os-js/osjs-server/pull/63

mahsashadi commented 2 years ago

Do you agree to split like this:

  1. URL serialization (done)
  2. Client option filtering (maybe should be considered in url serialization)
  3. Add VFS capabilities method
  4. Improve client sorting algorithm
andersevenrud commented 2 years ago

I'm closing this since we've agreed on a way to move forward.

I've also opened an issue where we can concentrate the discussion. Things got a bit confusing with the client/server stuff :D

https://github.com/os-js/OS.js/issues/804