Skip to content

Conversation

@thomashoneyman
Copy link
Contributor

This PR introduces the encode and decode functions for URIs and for x-www-form-urlencoded. It solves this comment wrt updating libraries to PureScript 0.14.

I've relied on the MDN documentation seen here:

I haven't yet added tests for these functions, but this PR at least allows us to bikeshed on implementation & naming.

@thomashoneyman thomashoneyman self-assigned this Dec 15, 2020
spago.dhall Outdated
{ name = "js-uri"
, dependencies = [ "console", "effect", "psci-support" ]
, dependencies =
[ "console", "effect", "functions", "maybe", "psci-support", "strings" ]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can drop the strings dependency if I apply replacements in JavaScript instead of in PureScript (for the *FormURLComponent functions).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against this. This library already has some non-trivial FFI bindings (well, they're only just non-trivial 😄) so doing it all in the FFI is fine to me.

@thomashoneyman
Copy link
Contributor Author

Once merged, I can make a release, add this to the 0.14 package set, and then:

  1. Deprecate the globals library
  2. Update affjax, uri, routing, and form-urlencoded to rely on this library instead of globals
  3. Create an issue to replace this library, one day, with an implementation in PureScript that doesn't rely on the FFI so that it can be reused by other backends.

@hdgarrood
Copy link

I didn’t realise that the longer term ambition is to have this library written in PureScript with no FFI; in this case I think the name js-uri is actually a bit misleading. Or at least, it will be once that happens.

@thomashoneyman
Copy link
Contributor Author

I don't think this library should be rewritten in PureScript, just that it would be ideal for some RFC 3896 encoding / decoding to exist which is not written using the FFI.

@JordanMartinez
Copy link
Contributor

I'm pretty sure this is just a stopgap solution so we can get v0.14.0 out the door.

@thomashoneyman
Copy link
Contributor Author

Unless someone disagrees, once I've added some tests and dropped the strings dependency I'd like to publish this library and use it to complete the 0.14 transition for all contrib libraries. I plan to do that sometime tomorrow unless someone voices dissent before then. Thanks!

@thomashoneyman thomashoneyman merged commit 5edccd3 into main Dec 16, 2020
@thomashoneyman thomashoneyman deleted the implementation branch December 16, 2020 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants