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

switch to native javascript bind #1423

Closed
wants to merge 1 commit into from
Closed

switch to native javascript bind #1423

wants to merge 1 commit into from

Conversation

david-fong
Copy link
Contributor

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

Uses component-bind.

New behaviour

Uses javascript's native function binding, which is supported everywhere (IE9 and up). I have no qualms if this PR is rejected for the sake of the stability of the code base / "if-it's-not-broken-don't-fix-it". My thinking here is that if socket.io no longer needs a simple dependency, it's best to get rid of it.

Other information (e.g. related issues)

This is an answer to one of my questions in #1417.

@david-fong
Copy link
Contributor Author

@darrachequesne The browser build on GitHub actions is failing:

Error:
Zuul tried to run tests in saucelabs, however no saucelabs credentials were provided.
See the zuul wiki (https://github.com/defunctzombie/zuul/wiki/Cloud-testing) for info on how to setup cloud testing.

Do you know how to fix it?

@darrachequesne
Copy link
Member

darrachequesne commented Dec 11, 2020

Yes, the build failing is "normal" because the Sauce Labs token is not attached to the build from a fork (so that this token is not leaked). Let me test it on a branch of the repo (here EDIT: build is OK!).

Regarding the component-bind dependency, I think that is indeed a legacy from IE 6/7/8 support.

Anyway, thanks a lot for digging into this 👍

darrachequesne pushed a commit that referenced this pull request Dec 11, 2020
The polyfill for the bind() method was necessary for IE6/7/8, which we
do not support anymore.

Related: https://caniuse.com/mdn-javascript_builtins_function_bind
@darrachequesne
Copy link
Member

Merged as 7bc7a7b.

I have only edited the package.json file, from "backo2": "^1.0.2" to "backo2": "~1.0.2"

Thanks a lot!

@darrachequesne darrachequesne added this to the 3.1.0 milestone Dec 11, 2020
@david-fong david-fong deleted the native-function-bind branch December 11, 2020 18:39
@darrachequesne darrachequesne modified the milestones: 3.1.0, 3.0.5 Jan 5, 2021
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

2 participants