Promisify the preload functions

[meiamsome]

If no one else minds, I would be interested in taking this on and converting the preloads to all use promises under the hood. I suggest this would involve adding loadXPromise(..) versions of each of the current loading functions and having the older loadX(...) functions be mapped onto them, perhaps along the lines of the following snippet, that should be able to keep backwards compatibility viable.

p5.prototype.loadX = function() {
  var ret = {};
  ret.promise = p5.loadXPromise.apply(this, arguments);
  ret.promise.then(function() {
    // Set properties on ret like before
  });
}

In the future, we could then deprecate the use of the return directly and prefer to use loadX().promise.then(ret => variable = ret) and after a long deprecation time we could convert to something like the following:

p5.prototype.loadX = function() {
  var ret =  p5.loadXPromise.apply(this, arguments);
  ret.promise = ret;
}

And then deprecate the use of .promise. I think this makes sense from a future standpoint where we get rid of the current loadX behaviour but that is a somewhat desirable feature. Perhaps it may even be a good idea to let preload return a promise. This would let users do

let file1, file2;
async function preload() {
  file1 = await loadJSON(...);
  file2 = await loadString(...);
}

Which would be relatively new user friendly I think. Browser support is getting there for async/await so I think this is actually viable in the near future.

It is notable, however that using await like this would actually cause the loads to be called one-by-one, so perhaps this is not the best solution.

[limzykenneth]

I was thinking about promisifying the loading functions since they already are promises underneath (using fetch) but I didn't get the time to try anything out. What you propose seem to me a good path to start exploring. A few notes:

The promisified function by convention (as per Bluebird) is to suffix Async, whether to follow this or not I don't have a strong opinion. Node.js also has a promisify function but instead of creating a new promisified function, it replaces the current one, with callback, with a promisified one using the same name.

We need to consider the case where the data that was loaded in would have the property promise thereby replaceing our own promise property.

The current way to deal with parallel await is to do await Promise.all(...) which I'm not sure is good enough usability-wise.

[meiamsome]

@limzykenneth I would say that the load prefix implies that it's asynchronous already, so I'm still not sold on needing the double implication of the Async suffix, but I don't see a better option at the moment. You are right that adding a new property is dangerous and will lead to issues, and it isn't that nice of a solution either.

I intend to start work on these with the Async suffix now that I've laid the groundwork with #2948 and #2949. I intend to keep the inputs to all functions the same (Including supporting callbacks as a legacy)

Perhaps, when p5.js gets its first major version we can switch them out so that the old style loadX functions are removed and loadXAsync is renamed loadX, but that's a question for further down the road.

[Spongman]

How about having the user's preload function optionally return a promise (and/or and array of promises). If a promise is returned then it's awaited before setup is called. That way, in the future, the user can just mark their preload as 'async' and 'await' their async methods within.

[meiamsome]

That was the plan, yes, I already have code to this end locally. However, I was also considering making the setup and draw functions also have support for this to make them consistent and allow for asynchronous things in the draw loop (Not recommended obviously, but there's definitely a case for it imo)

Is there a case for adding a loadKeys function (name definitely up for debate - perhaps parallelLoad or loadInParallel, functionally the same as Bluebird's Promise.props) where you would be able to do the following:

let v1, v2, v3;
async function preload() {
  { v1, v2, v3 } = await loadKeys({
    v1: loadJSONAsync('json'),
    v2: loadStringsAsync('strings'),
    v3: loadModelAsync('model')
  });
}

[meiamsome]

Actually, thinking about it, we can deprecate the whole preload mechanism once we have the preloads promisified as long as we allow setup() to return a Promise and honour that, right? I think that would actually make a lot of sense. so that we no longer have to do reference tracking on preload functions?

[Spongman]

i think it might be a bit of a leap to require beginners to grok the whole async thing off the bat. certainly it makes sense to support it for more advanced scenarios, but i think the just-throw-stuff-in-preload-and-we'll-take-care-of-it idea works well for those still learning the ropes. i'm guessing you're replacing the refcounting stuff with just a bucket of promises?

[meiamsome]

Yeah, you're right - preload is definitely simple, but the concept of var x = loadX is unsustainable (See issues with loadJSON causing confusion: #2290, #2207, #2154) and just bad practices. We should probably keep preload but if old-style loadX are deprecated then it just becomes a simple function that gets executed prior to setup, which should be fine.

[Spongman]

yeah those placeholder objects are definitely a little janky. something that could probably have been solved well with Symbol(). oh the joys of web compat... ;-)

[kjhollen]

assigning this bug to @meiamsome since there is a pending PR, just to indicate work is in progress. :)

[ksen0]

Closing as completed