Skip to content

Evaluate PSCi expressions in the browser#2199

Merged
paf31 merged 14 commits intomasterfrom
psci-browser
Jul 10, 2016
Merged

Evaluate PSCi expressions in the browser#2199
paf31 merged 14 commits intomasterfrom
psci-browser

Conversation

@paf31
Copy link
Copy Markdown
Contributor

@paf31 paf31 commented Jun 19, 2016

This is very rough right now, but it works, and here's a demo.

This assumes some client JavaScript code which understands the websocket protocol being used. I put this together as a quick experiment based on the Try PureScript code.

  • Handle the websocket close event
  • Limit to a single connected client?
  • Handle CTRL+C correctly
  • Add a --browser command line option
  • Serve static resources using Warp
  • Avoid sending compiled code over the websocket. The client can load it from the static web server.
  • Bundle all JS resources on startup
  • Re-bundle on :reload

Fixes #2142.

cc @artemyarulin I see you've been working on this too. We should combine efforts, if you're interested?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jun 23, 2016

@artemyarulin If you're still interested in working on this, I refactored my code to allow for the two backends (see the nodeBackend and browserBackend functions), so it should be possible to port your CLI code to build the Backend from the command line arguments (using either --browser <port> or --node-opts).

If you don't have time, that's totally fine obviously, but I wanted to let you know.

@artemyarulin
Copy link
Copy Markdown

Thanks, really nice, I keep eye on everything related to PSCi. Unfortunately I don't have time to work on it now, but I'll try to play with it and report if anything interesting pop up.

@artemyarulin
Copy link
Copy Markdown

purescript-repl-react-native
Just quickly played with remote REPL and it works with React Native just fine, I'm very happy 👍

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jun 23, 2016

Alright, that's much cooler than my demo. 😄 I'm glad it worked for you, thank you for testing it out.

@garyb
Copy link
Copy Markdown
Member

garyb commented Jun 23, 2016

That's so cool!

psci/Main.hs Outdated
, " return buffer.join('\\n');"
, "};"
, "window.onload = function() {"
, " var socket = new WebSocket('ws://0.0.0.0:9160');"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Must remember to fix this port.

@artemyarulin
Copy link
Copy Markdown

Can we put JS/HTML file into separate file? Now editing it is quiet difficult. And would be cool to have auto-reconnect logic

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jun 23, 2016

Yes, I think that's a good idea, although the plan was that you wouldn't need to edit the JS/HTML, and would instead edit some foreign module in your project.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jun 24, 2016

I should be able to finish this off tomorrow, but it's pretty much ready for review now.

@artemyarulin
Copy link
Copy Markdown

I'm happy to test, just ping me when it's needed. Curious though - why limit client to only one connection? Only for simplicity of the first version?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jun 24, 2016

I thought about allowing multiple connections, but the issue is that I need the browser to return its result for printing, and with multiple connected browsers, that might get confusing. We can look into it later though, it might be interesting for testing out things like webworkers.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jun 25, 2016

Ok, this is ready for review now. I've tested it quite a bit, and it works quite nicely, but any additional testing is definitely appreciated.

@artemyarulin I added support for multiple clients too, although I only show the result from the first browser to respond.

@artemyarulin
Copy link
Copy Markdown

Oh, wow, awesome! I'll test it during today/tomorrow

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 2, 2016

Could someone please review the code here, so that we can try to release it in 0.9.2?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 2, 2016

Clearing the Travis cache seems to have fixed the issue, but we might need to keep a separate cache (or periodically clear the existing one) on MacOS.

@garyb
Copy link
Copy Markdown
Member

garyb commented Jul 2, 2016

I'm probably not going to have any time to review until later in the week, sorry.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 2, 2016

Ok, no worries, thanks.

@artemyarulin
Copy link
Copy Markdown

I'm not qualified enough to check the code, but from the actual functionality - everything works just fine.

psci/Main.hs Outdated
length js `seq` return (mid, js)
Bundle.bundle input [] Nothing "PSCI"

-- TODO: use JMacro here?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't used JMacro, so I can't speak to its applicability here, but I have used file-embed successfully for this sort of thing before.

@hdgarrood
Copy link
Copy Markdown
Contributor

Looks good! 👍 I just watched the video, this looks really exciting!

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 3, 2016

@hdgarrood Thanks for the review, I'll polish this up some time this weekend.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 3, 2016

@hdgarrood How's this?

@kritzcreek
Copy link
Copy Markdown
Member

Code looks good to me! Would be nice if we could figure out how to test this.

@hdgarrood
Copy link
Copy Markdown
Contributor

Looks good. Just checking: are these new files going to be included in source distributions?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 4, 2016

@hdgarrood I put them under extra-source-files, so I think so?

@hdgarrood
Copy link
Copy Markdown
Contributor

Yeah, cool, thats what I was imagining. I guess I missed that change in the cabal file the first time I looked.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Jul 9, 2016

So shall I merge this then? Maybe we can make a 0.9.2 release soon, with this, @kritzcreek's psc-ide changes, and hopefully fixes for some of the minor issues in the 0.9.2 milestone?

@kritzcreek
Copy link
Copy Markdown
Member

I think what we have makes for a pretty nice 0.9.2

@paf31 paf31 merged commit 5452688 into master Jul 10, 2016
@paf31 paf31 deleted the psci-browser branch July 10, 2016 20:43
archaeron pushed a commit to archaeron/purescript that referenced this pull request Apr 6, 2017
* Initial work on evaluating PSCi expressions in the browser using websockets

* Use Warp, add shutdown handler

* Bundle all JS resources on startup

* Refactoring before supporting multiple backends

* Tidy up JavaScript component. Return output/exception stack to PSCi.

* Refactor to allow different backends

* Remove comments

* Add port option, fork ping thread

* Only allow one client

* Support multiple clients, handle reloads

* extra-deps

* stack.yaml

* Implement suggestions: use file-embed, save console.log, use case

* Add static files to bundle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote connection to PSCi (REPL)

5 participants