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

Change "const client of" to "let client of" #1056

Closed
wants to merge 1 commit into from

Conversation

colinligertwood
Copy link

All but the most recent versions of firefox explode on the
use of for (const variable of ...). Switching declaration to
let solves it.

My team discovered this after updating npm modules of a
project which makes use of Autobahn.js.

All but the most recent versions of firefox explode on the
use of for (const variable of ...). Switching declaration to
let solves it.

My team discovered this after updating npm modules of a
project which makes use of Autobahn.js.
@lpinca
Copy link
Member

lpinca commented Mar 30, 2017

I don't understand. This is made to run on Node.js so there is no need for this change. You can't run the WebSocket server in the browser anyway.

@colinligertwood
Copy link
Author

Understood. The issue I'm having is WebsocketServer is being bundled in my project with Autobahn.js for the browser, and then fails in Firefox, which until recently considered the for const of invalid.. If you'd rather keep the const, which I also agree is valid, I'll try and find another solution.

@lpinca
Copy link
Member

lpinca commented Mar 30, 2017

I find it hard to believe that Autobahn.js is bundling this lib for the browser. Can you show me where?
It seems that they are correctly excluding ws from the bundle.

@colinligertwood
Copy link
Author

Indeed. It's being improperly included in the build by Meteor which ignores 'browser' in package.json if it's an object, as is the case with Autobahn. It's a known Meteor issue. meteor/meteor#6890

As I said before, If you'd rather keep the const, I'll find another solution.

@lpinca
Copy link
Member

lpinca commented Mar 30, 2017

@colinligertwood I'm only trying to understand why this issue happens.
If this is a very edge case and there is a not too complex workaround I would prefer to keep const. If not feel free to reopen. The change is trivial and shouldn't cause any harm.

@colinligertwood
Copy link
Author

colinligertwood commented Mar 30, 2017

Thanks, I've managed to get around it for the time being by simply removing the ws library from the project. It results in a compile time warning, but it still builds and works fine. Since, as you pointed out, it shouldn't be loaded into the browser anyway, and the firefox issue is itself a bug that's been since fixed, I'm happy call it an edge case and work around it for a while.

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.

2 participants