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

Loader: Add "setFromFetchOptions" function #21286

Closed
wants to merge 3 commits into from

Conversation

gkjohnson
Copy link
Collaborator

Related issue: --

Description

This is a suggestion to add a setFromFetchOptions( fetchOptions ) function to Loader. fetch has become the predominant way to perform requests and it comes with a (in my opinion) more intuitive set of options for defining the request that is not supported by the three.js loaders. In my apps I carry around a common fetch options object that defines the "mode", "credentials", and necessary "headers" needed to make a request for a certain set of resources and it's not immediately clear how to map those options to the old XMLHttpRequest / three.js Loader model. setFromFetchOptions makes it easy to map the more modern fetch options to those needed by the loader ultimately calling setWithCredentials, setCrossOrigin, and setRequestHeader with ( as far as I can tell) the equivalent values.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 3, 2021

In my apps I carry around a common fetch options object that defines the "mode", "credentials", and necessary "headers" needed to make a request for a certain set of resources

You've made the PR based on this assumption but there are surely enough apps outside there which do not use such a workflow. Hence, setFromFetchOptions() seems to application specific to me and I would not add it to the generic Loader class.

@gkjohnson
Copy link
Collaborator Author

I never meant to imply this was more enabling for apps -- it's a convenience function that affords (in my opinion) a much more intuitive and ergonomic way to set request parameters on loaders and bring the API more up to par with the modern way to make requests (fetch).

At some point I'd think the FileLoader should be changed to use fetch under the hood (#22468) at which point using fetch options directly will be more natural.

@gkjohnson gkjohnson closed this Sep 7, 2021
@mrdoob
Copy link
Owner

mrdoob commented Sep 7, 2021

At some point I'd think the FileLoader should be changed to use fetch under the hood (#22468) at which point using fetch options directly will be more natural.

That sounds good to me. And caniuse seems to be decent. Anyone wants to give it a go?

@gkjohnson
Copy link
Collaborator Author

Anyone wants to give it a go?

Maybe @DefinitelyMaybe is interested in converting FileLoader to use fetch instead of XMLHTTPRequest considering #22468?

@DefinitelyMaybe
Copy link
Contributor

ooooooo, yeah, I'll look into it today

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.

4 participants