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

Add xhr prop #96

Merged
merged 3 commits into from
Sep 23, 2020
Merged

Add xhr prop #96

merged 3 commits into from
Sep 23, 2020

Conversation

ilyasakin
Copy link
Contributor

I have added xhr prop which you use when you need to stream like JWT protected content. https://github.com/goldfire/howler.js#xhr-object-null

@Stenerson
Copy link
Collaborator

Stenerson commented Sep 16, 2020

Thanks @iakindev! Ahh it looks like I merged the xhrWithCredentials PR (#77) right after Howler removed it 🤦

@andfk, not sure if you want to weigh in... 🤷‍♂️

I think I'll plan to merge this now and remove xhrWithCredentials in a future release. I don't love the duplication of withCredentials but it seems that the xhr prop allows more options that will be useful and at this point removing xhrWithCredentials becomes a breaking change (again 🤦)

I'll give it at least a day or so for people to weigh in. 🙏

@andfk
Copy link
Contributor

andfk commented Sep 16, 2020

@Stenerson 100% agree by using the xhr prop instead :)

Many thanks @iakindev

@ilyasakin
Copy link
Contributor Author

Sounds like everybody is agreed. So I thought I might as well remove xhrWithCredentials myself.
@Stenerson, if you worried about breaking change please feel free to not include latest commit to merge.

Thanks!

@Stenerson
Copy link
Collaborator

Thanks @iakindev actually I think this is ok - I'll plan to cut a new major release this weekend that requires >= 2.2.0 (the howler version that changed this functionality) and also incorporates these changes. Thanks both!

Side note: I wish Howler followed semver rather than including a known breaking change in 2.2.0 🤷‍♂️

@Stenerson Stenerson merged commit 9e0e0e2 into thangngoc89:master Sep 23, 2020
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.

None yet

3 participants