Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JS: Remove dead state transition validation code #1771

Merged
merged 1 commit into from
Oct 9, 2023
Merged

JS: Remove dead state transition validation code #1771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 5 additions & 17 deletions binderhub/static/js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
/* If this file gets over 200 lines of code long (not counting docs / comments), start using a framework
State transitions that are valid are:
start -> waiting
start -> built
start -> failed
waiting -> building
waiting -> failed
building -> pushing
building -> failed
pushing -> built
pushing -> failed
*/
import ClipboardJS from 'clipboard';
import 'event-source-polyfill';
Expand Down Expand Up @@ -54,7 +44,7 @@ function build(providerSpec, log, fitAddon, path, pathType) {
const buildEndpointUrl = new URL("build", new URL(BASE_URL, window.location.href));
const image = new BinderRepository(providerSpec, buildEndpointUrl, buildToken);

image.onStateChange('*', function(oldState, newState, data) {
image.onStateChange('*', function(data) {
if (data.message !== undefined) {
log.writeAndStore(data.message);
fitAddon.fit();
Expand Down Expand Up @@ -92,16 +82,14 @@ function build(providerSpec, log, fitAddon, path, pathType) {
image.close();
});

image.onStateChange('built', function(oldState) {
if (oldState === null) {
$('#phase-already-built').removeClass('hidden');
$('#phase-launching').removeClass('hidden');
}
image.onStateChange('built', function() {
$('#phase-already-built').removeClass('hidden');
$('#phase-launching').removeClass('hidden');
$('#phase-launching').removeClass('hidden');
Copy link
Member

Choose a reason for hiding this comment

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

This line is a duplicate

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for revieiwng @manics! I opened #1772 as I was too quick on the ball

updateFavicon(BASE_URL + "favicon_success.ico");
});

image.onStateChange('ready', function(oldState, newState, data) {
image.onStateChange('ready', function(data) {
image.close();
// If data.url is an absolute URL, it'll be used. Else, it'll be interpreted
// relative to current page's URL.
Expand Down
28 changes: 1 addition & 27 deletions js/packages/binderhub-client/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export class BinderRepository {
this.buildUrl.searchParams.append("build_token", buildToken);
}
this.callbacks = {};
this.state = null;
}

/**
Expand Down Expand Up @@ -128,39 +127,14 @@ export class BinderRepository {
}
}

/**
* @param {string} oldState Old state the building process was in
* @param {string} newState New state the building process is in
* @returns True if transition from oldState to newState is valid, False otherwise
*/
validateStateTransition(oldState, newState) {
if (oldState === "start") {
return (
newState === "waiting" || newState === "built" || newState === "failed"
);
} else if (oldState === "waiting") {
return newState === "building" || newState === "failed";
} else if (oldState === "building") {
return newState === "pushing" || newState === "failed";
} else if (oldState === "pushing") {
return newState === "built" || newState === "failed";
} else {
return false;
}
}

_changeState(state, data) {
[state, "*"].map(key => {
const callbacks = this.callbacks[key];
if (callbacks) {
for (let i = 0; i < callbacks.length; i++) {
callbacks[i](this.state, state || this.state, data);
callbacks[i](data);
}
}
});

if (state && this.validateStateTransition(this.state, state)) {
this.state = state;
}
}
}
Loading