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

Allow Loaders to require onLoad parameter #598

Merged
merged 1 commit into from
Sep 9, 2023

Conversation

Methuselah96
Copy link
Contributor

@Methuselah96 Methuselah96 commented Sep 9, 2023

Why

Some Loaders check to see if onLoad is undefined before calling it, some just call it without an undefined check. With the changes in #570, it was impossible to define a Loader where the onLoad parameter was required since the base Loader class required it be optional.

This came up in practice because the loader types defined by three-stdlib have some required onLoad parameters that were causing type errors. After further investigation, this seemed like the best solution both for @types/three and three-stdlib.

What

This PR allows for either required or optional onLoad parameters by making the onLoad parameter required in the base Loader class and then overloading the load method in any derived classes that want to make onLoad optional.

Checklist

  • Checked the target branch (current goes master, next goes dev)
  • Added myself to contributors table
  • Ready to be merged

@Methuselah96 Methuselah96 merged commit 0b0ba36 into master Sep 9, 2023
3 checks passed
@Methuselah96 Methuselah96 deleted the allow-required-loader-on-load branch September 9, 2023 19:48
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.

1 participant