Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

dates are ignored when building query strings #22

Open
AdamWillden opened this issue Aug 19, 2016 · 7 comments
Open

dates are ignored when building query strings #22

AdamWillden opened this issue Aug 19, 2016 · 7 comments

Comments

@AdamWillden
Copy link

AdamWillden commented Aug 19, 2016

I'm submitting a bug report

  • Library Version:
    1.0.0

Please tell us about your environment:

  • Operating System:
    Windows 10
  • Node Version:
    5.11.1
  • NPM Version:
    3.10.5
  • JSPM OR Webpack AND Version
    JSPM 0.16.39
  • Browser:
    All probably. Chrome 52.0.2743.116 m (64-bit)
  • Language:
    ESNext

Current behavior:
Dates are not made into parameters they are ignored. This is as a result of the if (typeof (value) === 'object') check on line 128 resolving true for dates.

Expected/desired behavior:
gist.run is down(??) so I can't show the behaviour currently.

  • What is the expected behavior?
    I believe line 128 should also check that it is not a date and let it fall through into the else block so that the date is present in the query string.
  • What is the motivation / use case for changing the behavior?
    I can't put dates directly in query strings, though I appreciate it's probably wise to format the date explicitly first into a string and will be my local work around/fix.
@EisenbergEffect
Copy link
Contributor

We'd be interested in a PR for this. It might make a nice contribution. If you do that, we probably need a pluggable way to change the dater (de)serialization.

@AdamWillden
Copy link
Author

Any ideas for how we could make it pluggable? I'm game to make a PR for this. Also, what do you think is the best method for making a suitable date string? this?

@EisenbergEffect
Copy link
Contributor

Just try something out and submit a PR, then we can go back and forth refining it, get a few more eyes on it, etc. and work it until we all have it where we want it. That's my recommendation.

@RWOverdijk
Copy link

👍 we have to work around this now. It's a matter of doing .toString and constructing the Date with the same string again when converting back.

@sylvain-hamel
Copy link

sylvain-hamel commented Dec 8, 2016

Both encodeURIComponent (used by aurelia-path) and jquery.params support dates. But they don't produce the same output.

encodeURIComponent
"Thu%20Dec%2008%202016%2000%3A40%3A02%20GMT-0500%20(Eastern%20Standard%20Time)"

$.param
"Thu+Dec+08+2016+00%3A36%3A00+GMT-0500+(Eastern+Standard+Time)"

ISO / JSON.stringify()
"2016-12-08T05:42:15.219Z"

I prefer ISO. This is how dates are serialized in JSON payloads and I don't know about other backends but a .Net WebApi would deserialize ISO dates with no extra effort.

@EisenbergEffect if you pick a format, we may be able to send in a PR.

@EisenbergEffect
Copy link
Contributor

What if we go with ISO but have a way to configure it with another implementation?

@Alexander-Taran
Copy link

@bigopon

looks like this wish came true.

result.push(`${encodeKey(key) }=${encode(value) }`);

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants