Long term planning for the Frontend #774

yuvipanda · 2019-01-23T17:48:47Z

Currently, we have one JS file + 1 HTML file (rendered with Jinja2) that does all the frontend work. This includes:

Client for the BinderHub API
Main page (when people see mybinder.org)
Loading page (when people go to a mybinder.org/v2/gh/user/repo launch URL)

(2) and (3) are particularly intertwined - they're restyled versions of the same page (HTML + JS).

In the future, I'd like us to improve on the following aspects:

JS Tests (we have none)
Export a BinderHub client API library that others can re-implement. Right now, everyone does their own implementation, and particularly run into problems against eventsource retries.
Implement both v2 (current) and newer API (whenever API refactor process #358 lands)
Split the main & loading pages
Make main page a lot more dynamic - ideally showcasing useful / featured repos & other forms of discovery
Better (privacy conscious) frontend instrumentation to see how people use the main page & how we can improve it.

Doing a refactor of the current JS in some form or way while it's still smallish (~400 lines) seems like a good idea to me!

Factors

Some factors to consider:

How much is it used inside the Jupyter community? We want to make it as easy as possible for other Jupyter contributors to work on this.
Does it help bring new people with different skillsets into the project? We should balance this against (1) - we wanna diversify who comes in and contributes to the project, and specific tech might help / hinder that.
How much is it used in the frontend dev world? We wanna use an approach that's likely to be well debugged & with a lot of resources out there on how to use it.
Is it appropriately scoped for our size? We shouldn't overcomplicate our lives by choosing tools meant for 100k lines projects while ours is much smaller.
Does it make working on the frontend a happy & joyful thing for many people in the long run? Often projects end up with convoluted untested frontend code that is hard to touch since it might break.

Options

Here are some options

Option 1: React + TypeScript

We can switch to using TypeScript as a language & use React for frontend components + tests.

Option 2: React + JS

Similar to above, but with just plain JS (JSX) instead of TypeScript.

Option 3: Next.js

Recommended by @betatim: https://nextjs.org/docs

Option 4: No React

We'll refactor and move away from monolithic single JS file, but keep using our current setup of pure DOM manipulation rather than use a library.

There are probably more options than this, but this should be a starting point to discuss!

yuvipanda · 2019-01-23T17:49:57Z

Fun fact - when I first started writing BinderHub, the very first line of JS I wrote was:

/* If this file gets over 200 lines of code long (not counting docs / comments), start using a framework

It's still there, and the file is over 400lines long now :)

yuvipanda · 2019-01-23T17:50:51Z

I personally really like TypeScript, and the JupyterLab / nteract communities seem to be using it heavily as well. However, @ellisonbg pointed out it might be overkill for us - and I agree.

yuvipanda · 2019-01-23T17:55:37Z

My current preference is to start with Option 4, and then see how it goes.

yuvipanda · 2019-01-23T17:56:19Z

/cc some people who might be interested: @minrk, @betatim, @ellisonbg, @ian-r-rose, @choldgraf, @willingc

sgibson91 · 2019-01-23T18:44:49Z

Hi @yuvipanda

I've often thought it would be nice to have some kind of checkbox on the main page for if you wanted to launch in a JupyterLab/RStudio environment. Rather than selecting URL path and type lab/rstudio as that's not overly clear to beginners. Just a thought 🙂

choldgraf · 2019-01-23T19:03:03Z

IMO option 4 will also make it easier to adopt options 1-3 in the future if we want to move in that direction, I'm +1 on option 4. It feels more like an iterative step rather than a total re-work.

put another way:

I think that we should do number 4 regardless. The question to me is "should we spend the extra time taking care of 1, 2, or 3 while we do number 4, or should we put off that work to a potential future time"

betatim · 2019-01-23T22:01:00Z

I'd vote for option 3. The main factor is that it used to be/is possible to use https://github.com/segmentio/create-next-app to get all the webpack/uglify/hot-reload/css shrinking etc stuff "for free". As someone who knows nothing about JS and web dev I enjoyed being taken by the hand and given something that looked useful, worked and wasn't super heavy weight. Since I setup bndr.it it seems things have changed so maybe next.js now has this built in??

https://nextjs.org/learn/ was very useful as a place to look up and (somewhat blindly) follow to get some good practices.

I'd split the "more dynamic front page" out from this refactor. Mostly because I think we can get some very easy wins there even if we don't make it dynamic. Having some popular repos shown, "syntactic sugar" like @sgibson91 mentioned, etc could be done already without any JS refactoring. Splitting this out means we could work on it in parallel and maybe pull in a new contributor?

choldgraf · 2019-01-23T22:07:46Z

If option 3 is "add no new features, just replicate the current structure of things using a simple framework anybody can use" then I'd be +1 on that. @betatim do you think that a person who has medium experience with HTML but no JS would be able to pick up next.js about as quickly as they'd pick up the current page setup?

betatim · 2019-01-23T22:17:37Z

For some more shameless lobbying: https://github.com/zeit/next.js/tree/master/examples is a good place to find out how to do stuff. So we can start lightweight but find inspiration and guidance. The recommendation right now is to start with an empty repo, follow nextjs.org/learn and pull stuff from the examples as needed.

I think on "do frontend devs use this" next is Ok and not a toolkit that is going away soon.

There are also react components we might be able to reuse from the nteract project (@rgbkrk tells me we should check out rx-binder + rx-jupyter)

betatim · 2019-01-23T22:20:24Z

I found it easy to build bndr.it with next.js, I find it very hard to understand our JS and DOM stuff on mybinder.org. However I never really got into using jQuery and went straight from writing HTML+CSS 15 years ago to react via a big break of doing "nothing web whatsoever".

I'd support the idea (no matter which option) to do the refactor without changing any functionality/looks/design. Then after the switch change design/functions/etc. It splits it into two smaller tasks. Smaller diffs to review, less potential for the refactor to get held up by discussion about changing looks/feel/functionality.

choldgraf · 2019-01-23T22:31:56Z

Sounds good - I am just trying to tease apart the level of difficulty that these tools will introduce for people of all technical backgrounds, not just those with web-dev experience. Y'all know more about this than I do, so I trust the community's input on this!

yuvipanda · 2019-01-23T22:35:19Z

I'm unsure what nextjs gives us over using react, but I guess these are implementation details...

I think we should definitely split this into two issues maybe - one for refactoring the current JS, and another for seeing if we should move to something.

rgbkrk · 2019-01-23T23:05:41Z

nextjs is still react, it mostly just gives you an opinionated scaffolding around the webpack so that you're (hopefully) not having to muck with all the JS config pain. Note: it can still be used with typescript if you want that.

captainsafia · 2019-01-24T21:29:33Z

Based on my experience, I think the best flow is Option 4 -> Option 2 -> Option 3.

The refactor in Option 4 should split up the contents of the single JS file into different files by UI element. For example, a file for repository input field, the file path input field, etc.

Once this refactor is complete, we can go through file by file and convert the set of jQuery events/actions into a React component. We don't have to opt-in to React completely. With the way that React is setup, we can bit by bit render more and more React components on the page as everything is migrated.

Once that is done, we can move over to Option 3 to leverage some of the performance improvements that Next.JS gives.

IMO, this process allows us to refactor and migrate things bit-by-bit and opt in to the tools as we need them. I'm happy to lead this effort, starting with a lightweight refactor so the site can continue to be deployed while we iterate towards our ultimate end goal.

rgbkrk · 2019-01-24T21:33:05Z

That sounds like a great way to make progress on this @captainsafia and allow others to help in the process iteratively. Feel free to ping me for review on PRs.

choldgraf · 2019-01-24T22:58:37Z

+1 to @captainsafia 's idea, thanks for the input!

yuvipanda · 2019-01-24T22:59:33Z

<3 @captainsafia would love for you to lead the effort!!!

minrk · 2019-01-27T00:36:46Z

That sounds like a great plan! Thanks to the js experts.

jtpio · 2019-12-30T10:49:46Z

Export a BinderHub client API library that others can re-implement. Right now, everyone does their own implementation, and particularly run into problems against eventsource retries.

A standalone @binderhub/client npm package would be really useful, that other (alternative) frontends to BinderHub could reuse. This client would abstract away the EventSource and other details, and could listen to events:

import BinderImage from '@binderhub/client';

var image = new BinderImage();
image.onStateChange('building', () => { ... });

Similar to:

binderhub/binderhub/static/js/index.js

Lines 168 to 225 in 23eeb43

    
             var image = new BinderImage(providerSpec); 
        
             image.onStateChange('*', function(oldState, newState, data) { 
        
               if (data.message !== undefined) { 
        
                 log.write(data.message); 
        
                 log.fit(); 
        
               } else { 
        
                 console.log(data); 
        
               } 
        
             }); 
        
             image.onStateChange('waiting', function(oldState, newState, data) { 
        
               $('#phase-waiting').removeClass('hidden'); 
        
             }); 
        
             image.onStateChange('building', function(oldState, newState, data) { 
        
               $('#phase-building').removeClass('hidden'); 
        
               log.show(); 
        
             }); 
        
             image.onStateChange('pushing', function(oldState, newState, data) { 
        
               $('#phase-pushing').removeClass('hidden'); 
        
             }); 
        
             image.onStateChange('failed', function(oldState, newState, data) { 
        
               $('#build-progress .progress-bar').addClass('hidden'); 
        
               $('#phase-failed').removeClass('hidden'); 
        
               $("#loader").addClass("paused"); 
        
               // If we fail for any reason, show an error message and logs 
        
               update_favicon(BASE_URL + "favicon_fail.ico"); 
        
               log.show(); 
        
               if ($('div#loader-text').length > 0) { 
        
                 $('#loader').addClass("error"); 
        
                 $('div#loader-text p.launching').html('Error loading ' + spec + '!<br /> See logs below for details.'); 
        
               } 
        
               image.close(); 
        
             }); 
        
             image.onStateChange('built', function(oldState, newState, data) { 
        
               if (oldState === null) { 
        
                 $('#phase-already-built').removeClass('hidden'); 
        
                 $('#phase-launching').removeClass('hidden'); 
        
               } 
        
               $('#phase-launching').removeClass('hidden'); 
        
               update_favicon(BASE_URL + "favicon_success.ico"); 
        
             }); 
        
             image.onStateChange('ready', function(oldState, newState, data) { 
        
               image.close(); 
        
               // user server is ready, redirect to there 
        
               image.launch(data.url, data.token, path, pathType); 
        
             }); 
        
             image.fetch(); 
        
             return image; 
        
           }

choldgraf · 2020-01-02T11:26:30Z

@jtpio I think there's general agreement that splitting up the mega JS file is a great idea (@captainsafia made some progress on this in #777). That PR has stalled, but perhaps we can look to it for inspiration? Or make some more progress on those items if @captainsafia is still interested? I personally am still very pro-this-idea, we all just have too many other things going on :-)

I think the example you give would also be useful for tools like Thebelab to utilize

rgbkrk · 2020-01-02T18:26:33Z

It looks like #777 was an issue tracking steps to take. Three were done in bite sized chunks, with several available for takers.

choldgraf · 2020-01-02T19:59:57Z

@rgbkrk ah you're right, for some reason my brain processed that as a PR 😊 (which makes way more sense... I was wondering why there was a PR that was referencing other bite sized PRs lol).

more bite-sized PRs like the ones on that issue to chip away at that would be great!

yuvipanda · 2023-10-09T08:04:40Z

I'm picking this back up :)

I think my first goal is to actually cleanup the @jupyterhub/binder-client enough to get it to 100% unit test coverage. I've a WIP PR for it, and it does lead to getting us out of some of the weird and buggy callback stuff we have going on.

willingc · 2023-10-09T12:31:51Z

@yuvipanda You may want to try Playwright for testing. Nice ergonomics and fast.

yuvipanda · 2023-10-09T17:43:34Z

Thanks for the pointer, @willingc! It does look pretty cool! I will pick it up when we get to doing end to end testing. Do you know if it can be used purely for unit testing? From looking at the docs, it looks like jest is still a better fit for unit testing while this is great for end to end.

Soooo, because `this.state` was initialised to `null` and not 'start', this code was always actually dead. `validateStateTransition` had no branch for `oldState` being null, so always returned `false`. This meant `this.state` was *never* set to current state, and so in all callbacks, `oldState` was always `null`! I validated this again by console.logging `oldState` and `newState` inside `validateStateTransition`. Here's the output for when a full build is performed: ``` null -> waiting null -> fetching null -> unknown null -> building null -> failed null -> built null -> launching ``` And for when launching a previously built image ``` null -> launching null -> ready ``` I also put one just above where `this.state` is set, and that code was never called. I was hoping to remove all this code anyway, as it doesn't actually validate nor test anything - that's really done by unit tests and integration tests. I thought this would hide bugs - but turns out, it was a bug itself, and hid nothing. This also changes the signature of the callback for `onChangeState`, dropping `oldState` and `newState`. `oldState` was always `null`, and in our code, we actually don't use `newState` *at all*. The one place that was conditional on `oldState` being null is now unconditional, because `oldState` was always `null`. Ref jupyterhub#774

JS now also supports writing async code with [Asnc Generators](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AsyncGenerator) which work very much like how async code would in Python. This allows us to write code that flows better, is more familiar to python folks, and avoids messy callback hell. This code is a *straight* port of the existing callback based code to use an Async Generator instead. The EventIterator library is a very helpful convenience function here, allowing us to translate the callbacks from EventSource into an async generator without having to write a queue of promises ourselves. The iterator is stopped exactly the same way the callbacks were earlier stopped (whenever .stop() is called). All the callbacks have been switched over to a switch / case statement instead. We can get cleaner and use exceptions to handle failures, instead of the 'failure' case, but that can come in a later PR so this is a purely mechanical refactoring from one async method to another. An appropriate test for this is also added. To fully test the EventSource flow, we create a temporary web server that will serve a real captured eventsource response, with 1ms gaps between lines to fully simulate how this would work in reality. This gives us much more confidence about how this would work than mocking. With this, @jupyterhub/binder-client now has 100% unit test coverage! Ref jupyterhub#774

minrk · 2023-10-10T12:09:11Z

I learned about playwright from @bl-aire and we now use it for the browser tests in jupyterhub. It's quite nice. I've since used it to script some tedious tasks I have to do for teaching with a web service that doesn't have an API.

I don't think it's really for unittesting, though. But since there's so little that our js does other than talking to BinderHub, maybe full browser tests are all we need?

willingc · 2023-10-10T14:13:46Z

More useful for component testing. Jest is fine for unit tests. IIRC playwright can execute jest tests too.

…URLs This is two changes that were easier to make together - BASE_URL and BADGE_BASE_URL are now URL objects rather than strings that are manipulated. With this done, we no longer use string manipulation for URLs anywhere! - Both BASE_URL and BADGE_BASE_URL are now always set, as we had a bunch of code that was using BADGE_BASE_URL if available but falls back to BASE_URL + origin if it was not set. This fallback is now implemented globally - The function to make links and generate badge markup is moved into `@jupyterhub/binderhub-client` as it is reasonably generic and not super specific to our frontend alone. This also involves them not reading BASE_URL and BADGE_BASE_URL globally, but having that information be passed in. Tests are also added here to catch any future issues that may arise. - Note for future fix - BADGE_BASE_URL is really PUBLIC_BASE_URL or similar, as it is used both for the location of the badge image (original intent) but also for the links we generate to share. This is relevant only for federation, where we want shared links to point to mybinder.org even though the API call itself may go to a specific member of the federation. I will do this deprecation + rename in a future PR so as to not make this PR bigger. Ref jupyterhub#774

…URLs This is two changes that were easier to make together - BASE_URL and BADGE_BASE_URL are now URL objects rather than strings that are manipulated. With this done, we no longer use string manipulation for URLs anywhere! - Both BASE_URL and BADGE_BASE_URL are now always set, as we had a bunch of code that was using BADGE_BASE_URL if available but falls back to BASE_URL + origin if it was not set. This fallback is now implemented globally, and correctly. - BASE_URL is also now always fully qualified, and BADGE_BASE_URL always was. - The function to make links and generate badge markup is moved into `@jupyterhub/binderhub-client` as it is reasonably generic and not super specific to our frontend alone. This also involves them not reading BASE_URL and BADGE_BASE_URL globally, but having that information be passed in. Tests are also added here to catch any future issues that may arise. - Note for future fix - BADGE_BASE_URL is really PUBLIC_BASE_URL or similar, as it is used both for the location of the badge image (original intent) but also for the links we generate to share. This is relevant only for federation, where we want shared links to point to mybinder.org even though the API call itself may go to a specific member of the federation. I will do this deprecation + rename in a future PR so as to not make this PR bigger. Ref jupyterhub#774

…URLs This is two changes that were easier to make together - BASE_URL and BADGE_BASE_URL are now URL objects rather than strings that are manipulated. With this done, we no longer use string manipulation for URLs anywhere! - Both BASE_URL and BADGE_BASE_URL are now always set, as we had a bunch of code that was using BADGE_BASE_URL if available but falls back to BASE_URL + origin if it was not set. This fallback is now implemented globally, and correctly. - BASE_URL is also now always fully qualified, and we document that the python code ensures it has a trailing slash always. - The function to make links and generate badge markup is moved into `@jupyterhub/binderhub-client` as it is reasonably generic and not super specific to our frontend alone. This also involves them not reading BASE_URL and BADGE_BASE_URL globally, but having that information be passed in. Tests are also added here to catch any future issues that may arise. - Note for future fix - BADGE_BASE_URL is really PUBLIC_BASE_URL or similar, as it is used both for the location of the badge image (original intent) but also for the links we generate to share. This is relevant only for federation, where we want shared links to point to mybinder.org even though the API call itself may go to a specific member of the federation. I will do this deprecation + rename in a future PR so as to not make this PR bigger. Ref jupyterhub#774

Some were using underscores, which is more python than JS Ref jupyterhub#774

- Optional args get [] around the param name, not the type! - Remove extra increased timeout for some jest tests, as they are not needed - Pass actual URL objects to some of the methods, rather than strings. These accidentally worked but shouldn't be relied upon. - Import just the function used during tests from node modules, rather than the entire module. Ref jupyterhub#774

- Optional args get [] around the param name, not the type! - Remove extra increased timeout for some jest tests, as they are not needed - Pass actual URL objects to some of the methods, rather than strings. These accidentally worked but shouldn't be relied upon. - Import just the function used during tests from node modules, rather than the entire module. - Fix type of `fetch` to indicate that's actually just an *iterator*, not a *generator*. Ref jupyterhub#774

sgibson91 mentioned this issue Jan 23, 2019

Figure out a way to track project outputs other than the book the-turing-way/the-turing-way#96

Open

captainsafia mentioned this issue Jan 25, 2019

Refactor index.js megafile into mutliple files #777

Closed

8 tasks

captainsafia mentioned this issue Jan 29, 2019

Refactor index.js #356

Closed

jtpio mentioned this issue Dec 30, 2019

BinderHub frontend package voila-gallery/voila-gallery.github.io#10

Open

yuvipanda mentioned this issue Oct 9, 2023

JS: Remove dead state transition validation code #1771

Merged

yuvipanda mentioned this issue Oct 10, 2023

JS: Use async generator instead of callbacks + add tests #1773

Merged

yuvipanda mentioned this issue Oct 16, 2023

JS: Refactor link & badge generation, use URLs (not string) for base URLs #1778

Merged

yuvipanda mentioned this issue Oct 20, 2023

JS: Remove all references to global URL constants #1781

Merged

yuvipanda added a commit to yuvipanda/binderhub that referenced this issue Oct 20, 2023

JS: Ensure all variables are camelCased

049ae2c

Some were using underscores, which is more python than JS Ref jupyterhub#774

yuvipanda mentioned this issue Oct 20, 2023

JS: Ensure all variables are camelCased #1782

Merged

yuvipanda mentioned this issue Oct 20, 2023

JS: Fix some JSDoc types, and misc details in tests #1783

Merged

This was referenced May 29, 2024

[EPIC] Allow selection of resources via URL parameters 2i2c-org/infrastructure#4151

Open

Shepherd a cleanup of the BinderHub UI 2i2c-org/infrastructure#4152

Open

Rewrite the frontend #1856

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Long term planning for the Frontend #774

Long term planning for the Frontend #774

yuvipanda commented Jan 23, 2019 •

edited

Loading

yuvipanda commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

sgibson91 commented Jan 23, 2019

choldgraf commented Jan 23, 2019 •

edited

Loading

betatim commented Jan 23, 2019

choldgraf commented Jan 23, 2019

betatim commented Jan 23, 2019

betatim commented Jan 23, 2019 •

edited

Loading

choldgraf commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

rgbkrk commented Jan 23, 2019

captainsafia commented Jan 24, 2019

rgbkrk commented Jan 24, 2019

choldgraf commented Jan 24, 2019

yuvipanda commented Jan 24, 2019

minrk commented Jan 27, 2019

jtpio commented Dec 30, 2019

choldgraf commented Jan 2, 2020 •

edited

Loading

rgbkrk commented Jan 2, 2020

choldgraf commented Jan 2, 2020 •

edited

Loading

yuvipanda commented Oct 9, 2023 •

edited

Loading

willingc commented Oct 9, 2023

yuvipanda commented Oct 9, 2023

minrk commented Oct 10, 2023

willingc commented Oct 10, 2023 •

edited

Loading

Long term planning for the Frontend #774

Long term planning for the Frontend #774

Comments

yuvipanda commented Jan 23, 2019 • edited Loading

Factors

Options

Option 1: React + TypeScript

Option 2: React + JS

Option 3: Next.js

Option 4: No React

yuvipanda commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

sgibson91 commented Jan 23, 2019

choldgraf commented Jan 23, 2019 • edited Loading

betatim commented Jan 23, 2019

choldgraf commented Jan 23, 2019

betatim commented Jan 23, 2019

betatim commented Jan 23, 2019 • edited Loading

choldgraf commented Jan 23, 2019

yuvipanda commented Jan 23, 2019

rgbkrk commented Jan 23, 2019

captainsafia commented Jan 24, 2019

rgbkrk commented Jan 24, 2019

choldgraf commented Jan 24, 2019

yuvipanda commented Jan 24, 2019

minrk commented Jan 27, 2019

jtpio commented Dec 30, 2019

choldgraf commented Jan 2, 2020 • edited Loading

rgbkrk commented Jan 2, 2020

choldgraf commented Jan 2, 2020 • edited Loading

yuvipanda commented Oct 9, 2023 • edited Loading

willingc commented Oct 9, 2023

yuvipanda commented Oct 9, 2023

minrk commented Oct 10, 2023

willingc commented Oct 10, 2023 • edited Loading

yuvipanda commented Jan 23, 2019 •

edited

Loading

choldgraf commented Jan 23, 2019 •

edited

Loading

betatim commented Jan 23, 2019 •

edited

Loading

choldgraf commented Jan 2, 2020 •

edited

Loading

choldgraf commented Jan 2, 2020 •

edited

Loading

yuvipanda commented Oct 9, 2023 •

edited

Loading

willingc commented Oct 10, 2023 •

edited

Loading