json-bgon: pull changes from urbit/urbit:#5877 #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As @xiphiness and I discussed out of band, these are the http-api changes from urbit/urbit#5877 forward-ported to be on top of the new, separate repo. See that PR for further discussion/context. Some of the questions raised therein remain open. Namely:
And of course, we may want to ship this as a separate library, or different opt-in mode, or whatever else seems appropriate, until such a time where we have the gall to make js developers write conversions themselves. |
|
@jalehman I think we need to get urbit/nockjs#1 merged first and this should use that. a good bit of mark's other comments here should also probably be addressed. @Fang- @xiphiness do y'all want to meet sometime soon about this? |
ilyakooo0
reviewed
Jun 5, 2023
src/Urbit.ts
Outdated
| @@ -97,7 +103,7 @@ export class Urbit { | |||
|
|
|||
| /** This is basic interpolation to get the channel URL of an instantiated Urbit connection. */ | |||
| private get channelUrl(): string { | |||
| return `${this.url}/~/channel/${this.uid}`; | |||
| return `${this.url}/~/channel-jam/${this.uid}`; | |||
| } | |||
|
|
|||
| private get fetchOptions(): any { | |||
There was a problem hiding this comment.
on line 111 we still have 'Content-Type': 'application/json', even though we are sending jammed nouns. Is this intentional?
There was a problem hiding this comment.
Good catch, that should become application/x-urb-jam.
Other parts of this PR are outdated as well (wrt the released version from urbit/urbit#6472), like the channel URLs now being the same as normal, and only that Content-Type header indicating the difference.
|
@xiphiness @Fang- @arthyn I am working on the same thing for Elm. I would like to join the conversations if possible. I am ~racfer-hattes. |
Since the nockjs api has changed lightly since the code herein was written, we have to make some small adjustments. Note that this also changes the interface for the .err() subscription interface callback, to bring it in line with the changed to the .event() interface we introduced during earlier cleanup.
We no longer use a special path (/~/channel-jam), but instead make use of request headers to indicate what mode to open a channel in, and how to interpret a channel event. This required loosening up the headers type a little bit, making changes to the fetchOptions, making them dynamic rather than static.
Previously, the poke() function was asynchronous on _both_ its PUT request _and_ the poke-n/ack. In the nack case, it would *reject* the latter promise instead of resolving it, causing the whole to throw an error. Due to implementation mishap, this would always throw undefined. Here, we update this behavior, so that it's asynchronous on _only_ the PUT request, and returns the event id as soon as that resolves. The callbacks passed as arguments still get called as normal. This misbehavior was discovered while finishing the implementation of the "handle poke nack" test, whose final form has been included. Note that this is a subtle breaking change. Clients who were awaiting on their poke() call will now see different/faster behavior, and no longer need to fold it into a catch block.
It includes the desk, so that mark files can be picked from the correct desk by eyre internally. This implementation detail ends up leaking into its interface right now, so we must account for it when unpacking the fact.
We were going out of and back into Atom shape, but we can just toString it in-place.
Replace json with nouns, make sure to test nouns for equality properly.
There were shenanigans aronud .eventSource getting recursively called into by .sendNounToChannel. This whole structure was unnecessarily recursive and hard to follow. Here we flatten it out a little bit, making .eventSource responsible for continuing after an initialization poke, in the "not yet initialized" case. This means that developers using this library need to explicitly call .eventSource() in order to establish a connection. This probably makes for a better API, although we should add a success/failure return value to it in the near future.
Eyre takes a list of commands for each incoming PUT. Here, we update sendNounToChannel() into sendNounsToChannel(), taking a variable number of arguments, which get converted into "real" Nouns inside of the function, using nockjs's dwim logic. (Of course, this means callers are still free to pass pre-built Nouns also.) This way, we don't end up with Noun construction boilerplate all over the place, and this lets us send the list of empty commands, which we want to use for channel initialization.
- renamed eventSource to connect - renamed authenticate to setupChannel - renamed connect to authenticate - no longer accept ship in the constructor - refactored constructor to take object instead of individual arguments - consistently follow patp syntax for ship and our - removed desk as a parameter and now require for thread - onReconnect removed in favor of onOpen, reconnect still signalled with param - various comment fixes and formatting
Instead of any, we can use Noun in many places. We also don't want/need to parameterize types as often as we used to. Also throws away unused types from types.ts.
Necessarily includes logic for turning the jambuffer in the response, into a proper Noun object. Only somewhat surprisingly, we need to reverse the byte order in the response, even though to accurately mock it, we don't need to reverse the bytes from the jam generated by nockjs. It would be most prudent to manually test this in the browser, to make sure we didn't trip ourselves up over endianness shenanigans here.
This adds a `fetch` option to the `Urbit` class, which allows you to pass in the local fetch implementation that works for you. Notably we did this to support React Native, which needs special fetch parameters. We also removed todos that were no longer relevant.
As currently written, the library really only supports the one mode. This is probably the way forward, too. So here we remove vestigial references to mode switching within the library.
This makes sure we re-export important functions from @urbit/nockjs. We also removed the URL prefixing which was causing issues with passing empty string as the URL. Finally, we reconfigured the setup so that the Urbit instance returned is immediately available and all interactions wait for the connection to be established.
Which just does a scry() call, but enforces a json mark, and handles the parsing of the ReadableStream for you.
Accept it as either pre-interpolated string, array of segments, or directly as a Noun. The conversion logic in subscribe() should maybe be factored out.
Now defaults to speaking raw nouns with spider, as supported through urbit/urbit#6943. Provides a jsonThread() fallback for speaking json. Doesn't yet support the "json one way, nouns the other" case. Factors out noun<->bytebuffer conversions into utility functions.
Make sure we use Path instead of flat strings where appropriate. Rename subscription callbacks, and unify callbacks into their "interface" types. Store those interface types, even if we strictly only need the callbacks themselves, it's useful data to keep around. We do remove the eventId from the callback interfaces though, you can't know them ahead of time, and client software is generally better off rolling their own identifiers if they really don't want to do anonymous callback function or similar. Updates tests to match.
Canonical aura formatting or bust.
This has the same kind of bignums that nockjs uses, letting us skip a double-conversion.
When initializing a channel connection, you may now optionally specify the channel mode, either 'noun' (default) or 'json'. This mode determines the shape of the responses that come back out of the library into your callbacks. Sending pokes may be done with either nouns or js data. Scries and threads retain their separate functions depending on what kind of response you want.
Typescript can't check these correctly, but we do want to give either a Noun or js-object depending on the channel mode. The function signatures should've been checked when they were first provided by the client.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Missing aura.js, need to figure this out. Also have not confirmed it builds.