Skip to content
This repository has been archived by the owner on Dec 18, 2020. It is now read-only.

Deprecate this library #22

Closed
hdgarrood opened this issue Oct 14, 2020 · 14 comments
Closed

Deprecate this library #22

hdgarrood opened this issue Oct 14, 2020 · 14 comments

Comments

@hdgarrood
Copy link
Contributor

This library is a random grab bag of various functions which happen to be in the global scope in JS. I don't think it makes a huge amount of sense in the first place (we shouldn't just copy the way JS organises things), and I think it especially doesn't make sense now that alternate backends are a thing.

I think the things that this library provides could more sensibly be split into other libraries:

  • Number values and operations like isNan, isFinite, infinity, parsing and formatting can live in a library like purescript-numbers (indeed most of them already do)
  • Parsing Ints is already provided in purescript-integers
  • encoding and decoding URIs and URI components could live in purescript-strings
  • I think now that we have safe versions of all of these, there's not really any reason for the unsafe versions to exist (other than perhaps that the Maybe return values for formatting numbers are awkward, but @sharkdp has already handled that in the way I think is most appropriate in the purescript-numbers library, by clamping the format parameters).

However, I think we should have functions for parsing and formatting Number values somewhere in core. So my proposal is to move things into other libraries as described above, move numbers into core, and then deprecate this library.

@sharkdp: would moving numbers to core be alright with you?

@hdgarrood
Copy link
Contributor Author

I forgot about purescript/purescript-numbers#8!

@JordanMartinez
Copy link
Contributor

I'm in agreement with this. While JS is the main backend, I don't think we should structure libraries around it as it does make things harder for alternate backends.

@JordanMartinez
Copy link
Contributor

I should have asked this first before working on purescript/purescript-numbers#11. Where do we need to port things in this repo once it's deprecated? There's a couple of parts to this repo:

  • all number related things: I assume purescript-numbers
  • unsafeStringify: perhaps purescript-strings?
  • encode/decode URI component: purescript-strings?

@garyb
Copy link
Member

garyb commented Oct 23, 2020

I'm not really convinced unsafeStringify should exist at all. It seems like a thing you'd only ever want to use in debugging, and purescript-debug already lets you print arbitrary values.

encodeURI / decodeURI / Component I don't feel belong in -strings. Assuming we're at all pretending -strings can be sensibly used cross-backend we're just adding some not-really-core-String-related stuff to it because it happens to exists in JS and we haven't thought of a better home for it. It belongs in some kind of URI related library really, but also, my experience with building purescript-uri tells me that neither variant of them actually conforms to the spec for URIs either, so I kinda have problems with them for that too. I certainly don't want to impose the use of purescript-uri on everyone either, because it is rather like using a sledgehammer to crack a nut most of the time, but I don't really know what to suggest.

@JordanMartinez
Copy link
Contributor

Since the URI things are more URI related, how about moving them outside of core and as a new repo in purescript-contrib or something?

@JordanMartinez
Copy link
Contributor

I'm not really convinced unsafeStringify should exist at all. It seems like a thing you'd only ever want to use in debugging, and purescript-debug already lets you print arbitrary values.

I agree with this.

Since the URI things are more URI related, how about moving them outside of core and as a new repo in purescript-contrib or something?

If this was done, what would the name of the repo be called? purescript-uri-components since purescript-uri is taken?

@garyb
Copy link
Member

garyb commented Oct 24, 2020

I'd lean towards purescript-js-uri or something like that I think, since it's a fairly specific implementation-related thing.

@JordanMartinez
Copy link
Contributor

Sounds fine to me. How should we create the repo? Is there some template directory I can clone and update? Or should I just reuse prelude?

@garyb
Copy link
Member

garyb commented Oct 24, 2020

Does the stuff in https://github.com/purescript-contrib/governance work for setting up new repos too? (If we're doing this in contrib as you suggested earlier). Or ask @thomashoneyman about it perhaps 🙂.

@hdgarrood
Copy link
Contributor Author

I didn’t think of those functions as JS-specific actually; aren’t they potentially useful to anyone processing URIs?

@thomashoneyman
Copy link
Contributor

Yea, that tool helps set up new projects too (so long as you’ve initialized a Spago project).

@garyb
Copy link
Member

garyb commented Oct 24, 2020

I didn’t think of those functions as JS-specific actually; aren’t they potentially useful to anyone processing URIs?

The specific behaviour implemented by the global encode/decodeURI/component is what I'm calling JS specific, since they just "do some kinda of URI encoding type thing", neither one of them encodes in a way that actually conforms to URI specs, and both of them require caveats depending on their use case: encodeURI cannot be used to encode proper GET or POST URIs for XHR. encodeURIComponent cannot be used to encode for application/x-www-form-urlencoded without doing a second replacement to substitute %20 for +. And so on.

@JordanMartinez
Copy link
Contributor

Now that the port PR has been merged, do we still want to create a purescript-js-uri repo? If so, that's the last action before we can close this issue.

@thomashoneyman
Copy link
Contributor

I plan to implement a js-uri repo tomorrow which uses the encodeURIComponent and decodeURIComponent functions, except a little bit stricter (compliant with RFC 3986), and supply additional functions if you specifically need x-www-form-urlencoded. From reading through specs it seems like form-urlencoded is an abomination and doesn't quite match with the RFC 3986 spec in the first place.

This at least unblocks a few libraries which can't otherwise update to 0.14.0-rc3, and it's a tiny dependency that's easy to drop if someone does get around to implementing an RFC 3986-compliant encode and decode pair without FFI.

I'll make an issue on the new repository for someone to write that function and for the library to be deprecated.

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

No branches or pull requests

4 participants