Skip to content

Add foundational support for loading p5 addons in preview-frame.ts. #63

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

Merged
merged 2 commits into from
Sep 11, 2016
Merged

Add foundational support for loading p5 addons in preview-frame.ts. #63

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffe5296
Add support for loading p5 addons in preview-frame.ts.
toolness Aug 28, 2016
1e23d92
Log script load errors.
toolness Sep 11, 2016
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
40 changes: 35 additions & 5 deletions lib/preview-frame.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,47 @@ require("../css/preview-frame.css");
interface PreviewFrameWindow extends PreviewFrame.Runner {
// This is exported by p5 when it's in global mode.
noLoop: () => void;

// This is the p5 constructor. An undocumented feature is that
// even the first argument is actually *optional*; if omitted,
// p5 will initialize itself in global mode.
p5: (sketch?: Function, node?: HTMLElement, sync?: boolean) => void;
}

let global = window as PreviewFrameWindow;

function loadP5(version: string, cb?: () => void) {
let url = '//cdnjs.cloudflare.com/ajax/libs/p5.js/' + version + '/p5.js';
function loadScript(url, cb?: () => void) {
Copy link

Choose a reason for hiding this comment

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

o hai :-) just a drive-by suggestion here: might be nice to listen for script load errors and log something to the console for debugging

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah good idea! I'll add that--this will also be useful for when we allow people to specify arbitrary URLs to third-party add-on libraries, since we want to provide good feedback if they misspell any of those URLs or something.

Copy link

Choose a reason for hiding this comment

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

Cool! Glad my drive-by review was helpful ^_^

On Monday, August 29, 2016, Atul Varma notifications@github.com wrote:

In lib/preview-frame.ts
#63 (comment):

}

let global = window as PreviewFrameWindow;

-function loadP5(version: string, cb?: () => void) {

  • let url = '//cdnjs.cloudflare.com/ajax/libs/p5.js/' + version + '/p5.js';
    +function loadScript(url, cb?: () => void) {

Ah good idea! I'll add that--this will also be useful for when we allow
people to specify arbitrary URLs to third-party add-on libraries, since we
want to provide good feedback if they misspell any of those URLs or
something.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/toolness/p5.js-widget/pull/63/files/ffe52963c21320e73664190dcfa70c2878b3a50e#r76695989,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAF4jGGEq79d0FNqZtg7XCRb6v1kKU5tks5qk1ZqgaJpZM4Ju51N
.

let script = document.createElement('script');

cb = cb || (() => {});

script.onload = cb;
script.onerror = () => {
console.log("Failed to load script: " + url);
};
script.setAttribute('src', url);

document.body.appendChild(script);
}

function loadScripts(urls: string[], cb?: () => void) {
Copy link

Choose a reason for hiding this comment

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

👍 you could get fancy with promises, but looping over the scripts and loading each should get the job done

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, I really like promises, but I'm not actually sure if we include the polyfill in the preview-frame... and I'd like to keep the preview frame as light as possible in terms of external dependencies and stuff, hrm. Lemme look into it!

cb = cb || (() => {});

let i = 0;
let loadNextScript = () => {
if (i === urls.length) {
return cb();
}
loadScript(urls[i++], loadNextScript);
};

loadNextScript();
}

function p5url(version: string) {
return `//cdnjs.cloudflare.com/ajax/libs/p5.js/${version}/p5.js`;
Copy link

Choose a reason for hiding this comment

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

might be worth it to allow a custom URL for self-hosting?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, this is actually filed in #48 actually--I'd prefer to keep this PR nice and small, and add support for that in a future one. 😄

}

function LoopChecker(sketch: string, funcName: string, maxRunTime: number) {
let self = {
wasTriggered: false,
Expand Down Expand Up @@ -92,9 +117,14 @@ function startSketch(sketch: string, p5version: string, maxRunTime: number,
errorCb(message, line);
});

document.body.appendChild(sketchScript);

loadP5(p5version);
loadScripts([
p5url(p5version),
], () => {
document.body.appendChild(sketchScript);
if (document.readyState === 'complete') {
new global.p5();
}
});
}

global.startSketch = startSketch;