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

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) }`);

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

No branches or pull requests

5 participants