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.

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

return XMLHttpRequest from load() methods that do not return anything else #8099

Closed
wants to merge 1 commit into from

Conversation

makc
Copy link
Contributor

@makc makc commented Feb 9, 2016

Follow-up PR to #8091 I skipped some loaders where it was not obvious to me where to stick that return statement, and also things like XxxTextureLoader that already return textures from load() call.

@mrdoob
Copy link
Owner

mrdoob commented Feb 10, 2016

Hmm, that's not very readable... Do you mind changing it to this pattern?

loader.load( url, function( text ) {

    onLoad( scope.parse( text ) );

}, onProgress, onError );

return loader;

@makc
Copy link
Contributor Author

makc commented Feb 10, 2016

return loader will not give you access to request

@mrdoob
Copy link
Owner

mrdoob commented Feb 11, 2016

Oh, true. How about...

var request = loader.load( url, function( text ) {

    onLoad( scope.parse( text ) );

}, onProgress, onError );

return request;

@makc
Copy link
Contributor Author

makc commented Feb 14, 2016

Right, now I need to figure out how to mass-replace that.

pixelnerve added a commit to toddlekan/unesco that referenced this pull request Apr 26, 2016
@gabrielcramer
Copy link

You may find this PR interesting. #9600
It allows you to abort all active requests using the LoadingManager.
To abort a single request we may store the current request inside every Loader.

@makc
Copy link
Contributor Author

makc commented Sep 28, 2016

This branch has conflicts

too late, closing.

@makc makc closed this Sep 28, 2016
@makc makc deleted the make-loaders-to-return-request branch February 5, 2017 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants